| Message ID | 20220527203651.258780-1-arbab@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] libstb: Fix memcpy overread in fakenv_readpublic() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-Docker_builds_and_checks | fail | check_build (ubuntu-rolling) failed at step Create Docker image. |
On Fri, May 27, 2022 at 03:36:51PM -0500, Reza Arbab wrote: >Caught by `make check` on fedora-rawhide (GCC 12): > > libstb/secvar/test/../storage/fakenv_ops.c: In function 'fakenv_readpublic': > libstb/secvar/test/../storage/fakenv_ops.c:155:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread] > 155 | memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(TPM2B_NAME)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > In file included from libstb/secvar/test/secvar-test-secboot-tpm.c:5: > libstb/secvar/test/../storage/secboot_tpm.c:35:15: note: source object 'tpmnv_vars_name' of size 34 > 35 | const uint8_t tpmnv_vars_name[] = { > | ^~~~~~~~~~~~~~~ > libstb/secvar/test/../storage/fakenv_ops.c:158:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread] > 158 | memcpy(&nv_name->t.name, tpmnv_control_name, sizeof(TPM2B_NAME)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > libstb/secvar/test/../storage/secboot_tpm.c:41:15: note: source object 'tpmnv_control_name' of size 34 > 41 | const uint8_t tpmnv_control_name[] = { > | ^~~~~~~~~~~~~~~~~~ > >The source and destination of each memcpy have known sizes, and we are >copying the smaller buffer into the larger one, so change the memcpy >size to that of the smaller buffer. Applied to master.
diff --git a/libstb/secvar/storage/fakenv_ops.c b/libstb/secvar/storage/fakenv_ops.c index 224ac2a5de7d..07ab98996cfb 100644 --- a/libstb/secvar/storage/fakenv_ops.c +++ b/libstb/secvar/storage/fakenv_ops.c @@ -152,10 +152,10 @@ static int fakenv_readpublic(TPMI_RH_NV_INDEX index, TPMS_NV_PUBLIC *nv_public, switch (index) { case SECBOOT_TPMNV_VARS_INDEX: - memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(TPM2B_NAME)); + memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(tpmnv_vars_name)); break; case SECBOOT_TPMNV_CONTROL_INDEX: - memcpy(&nv_name->t.name, tpmnv_control_name, sizeof(TPM2B_NAME)); + memcpy(&nv_name->t.name, tpmnv_control_name, sizeof(tpmnv_control_name)); break; default: return OPAL_INTERNAL_ERROR;
Caught by `make check` on fedora-rawhide (GCC 12): libstb/secvar/test/../storage/fakenv_ops.c: In function 'fakenv_readpublic': libstb/secvar/test/../storage/fakenv_ops.c:155:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread] 155 | memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(TPM2B_NAME)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from libstb/secvar/test/secvar-test-secboot-tpm.c:5: libstb/secvar/test/../storage/secboot_tpm.c:35:15: note: source object 'tpmnv_vars_name' of size 34 35 | const uint8_t tpmnv_vars_name[] = { | ^~~~~~~~~~~~~~~ libstb/secvar/test/../storage/fakenv_ops.c:158:17: error: 'memcpy' reading 134 bytes from a region of size 34 [-Werror=stringop-overread] 158 | memcpy(&nv_name->t.name, tpmnv_control_name, sizeof(TPM2B_NAME)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ libstb/secvar/test/../storage/secboot_tpm.c:41:15: note: source object 'tpmnv_control_name' of size 34 41 | const uint8_t tpmnv_control_name[] = { | ^~~~~~~~~~~~~~~~~~ The source and destination of each memcpy have known sizes, and we are copying the smaller buffer into the larger one, so change the memcpy size to that of the smaller buffer. Signed-off-by: Reza Arbab <arbab@linux.ibm.com> --- libstb/secvar/storage/fakenv_ops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)