Change in ...coreboot[master]: arch/x86: Use ssize_t to store length

[Jacob Garber (Code Review)]

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33962

Change subject: arch/x86: Use ssize_t to store length
......................................................................

arch/x86: Use ssize_t to store length

size_t is the wrong type to store the return value of
acpi_device_path_fill(), since any negative error values will be
converted to a very large unsigned integer and potentially cause
buffer overflow.

Change-Id: Ia8ed62ecfac8eaa18a61545bd203b3c7a7cd9ca5
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1402095
---
M src/arch/x86/acpi_device.c
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/33962/1

diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c
index 57fbc89..0fb8f3b 100644
--- a/src/arch/x86/acpi_device.c
+++ b/src/arch/x86/acpi_device.c
@@ -148,7 +148,7 @@
 const char *acpi_device_path_join(struct device *dev, const char *name)
 {
    static char buf[DEVICE_PATH_MAX] = {};
-	size_t len;
+	ssize_t len;

if (!dev)
    	return NULL;

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1: Code-Review+2

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1:

This one is bad

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has removed a vote on this change.

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Removed Code-Review+2 by HAOUAS Elyes

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1:

can't see an error [-Werror=sign-compare] on 'acpi_device.c' here : https://qa.coreboot.org/job/coreboot-gerrit/97548/consoleFull

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1:
> Patch Set 1:
>
> can't see an error [-Werror=sign-compare] on 'acpi_device.c' here : https://qa.coreboot.org/job/coreboot-gerrit/97548/consoleFull

I think -Wtype-limits is more in line with what you're looking for, but that doesn't catch it since <= 0 is still valid for unsigned types. -Wconversion would catch it though

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

Patch Set 1: Code-Review+1

Change in coreboot[master]: arch/x86: Use ssize_t to store length

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33962 )

Change subject: arch/x86: Use ssize_t to store length
......................................................................

arch/x86: Use ssize_t to store length

size_t is the wrong type to store the return value of
acpi_device_path_fill(), since any negative error values will be
converted to a very large unsigned integer and potentially cause
buffer overflow.

Change-Id: Ia8ed62ecfac8eaa18a61545bd203b3c7a7cd9ca5
Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1402095
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33962
Tested-by: build bot (Jenkins) 
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Paul Menzel 
---
M src/arch/x86/acpi_device.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/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c
index 57fbc89..0fb8f3b 100644
--- a/src/arch/x86/acpi_device.c
+++ b/src/arch/x86/acpi_device.c
@@ -148,7 +148,7 @@
 const char *acpi_device_path_join(struct device *dev, const char *name)
 {
    static char buf[DEVICE_PATH_MAX] = {};
-	size_t len;
+	ssize_t len;

if (!dev)
    	return NULL;