[Skiboot] [PATCH] libstb: Fix memcpy overread in fakenv_readpublic()
Reza Arbab
arbab at linux.ibm.com
Thu May 26 06:29:22 AEST 2022
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[] = {
| ^~~~~~~~~~~~~~~~~~
Using the first error as an example; there are a couple of things to
fix here. The types involved in the memcpy are
const uint8_t tpmnv_vars_name[34];
TPM2B_NAME *nv_name;
So the current memcpy line was likely intended to be
memcpy(&nv_name->t.name, tpmnv_vars_name, sizeof(nv_name->t.name));
Second, as the error indicates, the memcpy size should be no more than
the the size of the source or the destination, so
memcpy(&nv_name->t.name, tpmnv_vars_name,
MIN(sizeof(nv_name->t.name), sizeof(tpmnv_vars_name)));
Fix both errors similarly.
Signed-off-by: Reza Arbab <arbab at linux.ibm.com>
---
libstb/secvar/storage/fakenv_ops.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libstb/secvar/storage/fakenv_ops.c b/libstb/secvar/storage/fakenv_ops.c
index 224ac2a5de7d..35d9dcdde5c0 100644
--- a/libstb/secvar/storage/fakenv_ops.c
+++ b/libstb/secvar/storage/fakenv_ops.c
@@ -152,10 +152,12 @@ 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,
+ MIN(sizeof(nv_name->t.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,
+ MIN(sizeof(nv_name->t.name), sizeof(tpmnv_control_name)));
break;
default:
return OPAL_INTERNAL_ERROR;
--
2.31.1
More information about the Skiboot
mailing list