Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34374 )
Change subject: mb/getac/p470: Null-terminate ec_id string buffer ......................................................................
mb/getac/p470: Null-terminate ec_id string buffer
The EC ID of the ECDT needs to be null-terminated (see ACPI specification, section 5.2.15), which currently isn't being done due to an off-by-one error. strncpy() is bug-prone exactly because of issues like this, so just skip it entirely and use memcpy() instead.
Change-Id: I0b62e1f32177c9768fa978053ab26bca93d7248d Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1402104 --- M src/mainboard/getac/p470/acpi_tables.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/34374/1
diff --git a/src/mainboard/getac/p470/acpi_tables.c b/src/mainboard/getac/p470/acpi_tables.c index 57e2911..59c2876 100644 --- a/src/mainboard/getac/p470/acpi_tables.c +++ b/src/mainboard/getac/p470/acpi_tables.c @@ -70,7 +70,7 @@
ecdt->gpe_bit = 23; // SCI interrupt within GPEx_STS
- strncpy((char *)ecdt->ec_id, ec_id, strlen(ec_id)); + memcpy(ecdt->ec_id, ec_id, sizeof(ec_id));
header->checksum = acpi_checksum((void *) ecdt, ecdt_len);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34374 )
Change subject: mb/getac/p470: Null-terminate ec_id string buffer ......................................................................
Patch Set 1: Code-Review+1
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34374 )
Change subject: mb/getac/p470: Null-terminate ec_id string buffer ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34374 )
Change subject: mb/getac/p470: Null-terminate ec_id string buffer ......................................................................
mb/getac/p470: Null-terminate ec_id string buffer
The EC ID of the ECDT needs to be null-terminated (see ACPI specification, section 5.2.15), which currently isn't being done due to an off-by-one error. strncpy() is bug-prone exactly because of issues like this, so just skip it entirely and use memcpy() instead.
Change-Id: I0b62e1f32177c9768fa978053ab26bca93d7248d Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1402104 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34374 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: HAOUAS Elyes ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi_tables.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve HAOUAS Elyes: Looks good to me, approved
diff --git a/src/mainboard/getac/p470/acpi_tables.c b/src/mainboard/getac/p470/acpi_tables.c index 57e2911..59c2876 100644 --- a/src/mainboard/getac/p470/acpi_tables.c +++ b/src/mainboard/getac/p470/acpi_tables.c @@ -70,7 +70,7 @@
ecdt->gpe_bit = 23; // SCI interrupt within GPEx_STS
- strncpy((char *)ecdt->ec_id, ec_id, strlen(ec_id)); + memcpy(ecdt->ec_id, ec_id, sizeof(ec_id));
header->checksum = acpi_checksum((void *) ecdt, ecdt_len);
-
HAOUAS Elyes (Code Review) -
Jacob Garber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)