[PATCH v1] acpi_table: Fix coverity defect in acpi_write_spcr
Fix "Integer handling issues (SIGN_EXTENSION)" in newly added code: Cast serial_info.reg_offset to u64 to prevent an integer overflow when shifted too many bits to the left. Currently this never happens as the shift is supposed to be less than 4.
Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- lib/acpi/acpi_table.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index 6473d95c10..b9e12228fd 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -420,7 +420,7 @@ int acpi_write_dbg2_pci_uart(struct acpi_ctx *ctx, struct udevice *dev, static int acpi_write_spcr(struct acpi_ctx *ctx, const struct acpi_writer *entry) { struct serial_device_info serial_info = {0}; - ulong serial_address, serial_offset; + u64 serial_address, serial_offset; struct acpi_table_header *header; struct acpi_spcr *spcr; struct udevice *dev; @@ -473,7 +473,7 @@ static int acpi_write_spcr(struct acpi_ctx *ctx, const struct acpi_writer *entry }
serial_width = serial_info.reg_width * 8; - serial_offset = serial_info.reg_offset << serial_info.reg_shift; + serial_offset = ((u64)serial_info.reg_offset) << serial_info.reg_shift; serial_address = serial_info.addr + serial_offset;
/* Encode register access size */
On Mon, Oct 28, 2024 at 1:09 AM Patrick Rudolph patrick.rudolph@9elements.com wrote:
Fix "Integer handling issues (SIGN_EXTENSION)" in newly added code: Cast serial_info.reg_offset to u64 to prevent an integer overflow when shifted too many bits to the left. Currently this never happens as the shift is supposed to be less than 4.
Reviewed-by: Moritz Fischer moritzf@google.com
Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com
lib/acpi/acpi_table.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c index 6473d95c10..b9e12228fd 100644 --- a/lib/acpi/acpi_table.c +++ b/lib/acpi/acpi_table.c @@ -420,7 +420,7 @@ int acpi_write_dbg2_pci_uart(struct acpi_ctx *ctx, struct udevice *dev, static int acpi_write_spcr(struct acpi_ctx *ctx, const struct acpi_writer *entry) { struct serial_device_info serial_info = {0};
ulong serial_address, serial_offset;
u64 serial_address, serial_offset; struct acpi_table_header *header; struct acpi_spcr *spcr; struct udevice *dev;@@ -473,7 +473,7 @@ static int acpi_write_spcr(struct acpi_ctx *ctx, const struct acpi_writer *entry }
serial_width = serial_info.reg_width * 8;
serial_offset = serial_info.reg_offset << serial_info.reg_shift;
serial_offset = ((u64)serial_info.reg_offset) << serial_info.reg_shift; serial_address = serial_info.addr + serial_offset; /* Encode register access size */-- 2.46.2
On Mon, 28 Oct 2024 09:08:35 +0100, Patrick Rudolph wrote:
Fix "Integer handling issues (SIGN_EXTENSION)" in newly added code: Cast serial_info.reg_offset to u64 to prevent an integer overflow when shifted too many bits to the left. Currently this never happens as the shift is supposed to be less than 4.
Applied to u-boot/master, thanks!
On Mon, Oct 28, 2024 at 05:27:14PM -0600, Tom Rini wrote:
On Mon, 28 Oct 2024 09:08:35 +0100, Patrick Rudolph wrote:
Fix "Integer handling issues (SIGN_EXTENSION)" in newly added code: Cast serial_info.reg_offset to u64 to prevent an integer overflow when shifted too many bits to the left. Currently this never happens as the shift is supposed to be less than 4.
Applied to u-boot/master, thanks!
"I'll just bend my rules and apply this before release, it'll be fine" I said to myself. But: sandbox: + sandbox +(sandbox) In file included from include/linux/printk.h:4, +(sandbox) from include/linux/kernel.h:5, +(sandbox) from include/linux/libfdt_env.h:13, +(sandbox) from include/linux/libfdt.h:6, +(sandbox) from include/fdtdec.h:17, +(sandbox) from include/dm/ofnode.h:11, +(sandbox) from include/dm/device.h:13, +(sandbox) from include/dm.h:13, +(sandbox) from lib/acpi/acpi_table.c:10: +(sandbox) lib/acpi/acpi_table.c: In function ‘acpi_write_spcr’: +(sandbox) lib/acpi/acpi_table.c:498:15: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 8 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=] +(sandbox) 498 | debug("UART type %u @ %lx\n", spcr->interface_type, serial_address); +(sandbox) | ^~~~~~~~~~~~~~~~~~~~~~ +(sandbox) include/log.h:159:21: note: in definition of macro ‘pr_fmt’ +(sandbox) 159 | #define pr_fmt(fmt) fmt +(sandbox) | ^~~ +(sandbox) include/log.h:260:17: note: in expansion of macro ‘log’ +(sandbox) 260 | log(LOG_CATEGORY, \ +(sandbox) | ^~~ +(sandbox) include/log.h:282:9: note: in expansion of macro ‘debug_cond’ +(sandbox) 282 | debug_cond(_DEBUG, fmt, ##args) +(sandbox) | ^~~~~~~~~~ +(sandbox) lib/acpi/acpi_table.c:498:9: note: in expansion of macro ‘debug’ +(sandbox) | ^~~~~ +(sandbox) lib/acpi/acpi_table.c:498:33: note: format string is defined here +(sandbox) | ~~^ +(sandbox) | | +(sandbox) | long unsigned int +(sandbox) | %llx +(sandbox) cc1: all warnings being treated as errors +(sandbox) make[3]: *** [scripts/Makefile.build:256: lib/acpi/acpi_table.o] Error 1 +(sandbox) make[2]: *** [scripts/Makefile.build:398: lib/acpi] Error 2 +(sandbox) make[1]: *** [Makefile:1914: lib] Error 2 +(sandbox) make: *** [Makefile:177: sub-make] Error 2
And most other platforms too. I'm reverting this for now, and if someone is packaging v2025.01-rc1 too, just do that too. Sorry.
Hi Tom, On Tue, Oct 29, 2024 at 3:53 AM Tom Rini trini@konsulko.com wrote:
On Mon, Oct 28, 2024 at 05:27:14PM -0600, Tom Rini wrote:
On Mon, 28 Oct 2024 09:08:35 +0100, Patrick Rudolph wrote:
Fix "Integer handling issues (SIGN_EXTENSION)" in newly added code: Cast serial_info.reg_offset to u64 to prevent an integer overflow when shifted too many bits to the left. Currently this never happens as the shift is supposed to be less than 4.
Applied to u-boot/master, thanks!
"I'll just bend my rules and apply this before release, it'll be fine" I said to myself. But: sandbox: + sandbox +(sandbox) In file included from include/linux/printk.h:4, +(sandbox) from include/linux/kernel.h:5, +(sandbox) from include/linux/libfdt_env.h:13, +(sandbox) from include/linux/libfdt.h:6, +(sandbox) from include/fdtdec.h:17, +(sandbox) from include/dm/ofnode.h:11, +(sandbox) from include/dm/device.h:13, +(sandbox) from include/dm.h:13, +(sandbox) from lib/acpi/acpi_table.c:10: +(sandbox) lib/acpi/acpi_table.c: In function ‘acpi_write_spcr’: +(sandbox) lib/acpi/acpi_table.c:498:15: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 8 has type ‘u64’ {aka ‘long long unsigned int’} [-Werror=format=] +(sandbox) 498 | debug("UART type %u @ %lx\n", spcr->interface_type, serial_address); +(sandbox) | ^~~~~~~~~~~~~~~~~~~~~~ +(sandbox) include/log.h:159:21: note: in definition of macro ‘pr_fmt’ +(sandbox) 159 | #define pr_fmt(fmt) fmt +(sandbox) | ^~~ +(sandbox) include/log.h:260:17: note: in expansion of macro ‘log’ +(sandbox) 260 | log(LOG_CATEGORY, \ +(sandbox) | ^~~ +(sandbox) include/log.h:282:9: note: in expansion of macro ‘debug_cond’ +(sandbox) 282 | debug_cond(_DEBUG, fmt, ##args) +(sandbox) | ^~~~~~~~~~ +(sandbox) lib/acpi/acpi_table.c:498:9: note: in expansion of macro ‘debug’ +(sandbox) | ^~~~~ +(sandbox) lib/acpi/acpi_table.c:498:33: note: format string is defined here +(sandbox) | ~~^ +(sandbox) | | +(sandbox) | long unsigned int +(sandbox) | %llx +(sandbox) cc1: all warnings being treated as errors +(sandbox) make[3]: *** [scripts/Makefile.build:256: lib/acpi/acpi_table.o] Error 1 +(sandbox) make[2]: *** [scripts/Makefile.build:398: lib/acpi] Error 2 +(sandbox) make[1]: *** [Makefile:1914: lib] Error 2 +(sandbox) make: *** [Makefile:177: sub-make] Error 2
Appologies, I only build tested it on a single board. Will send an updated version.
And most other platforms too. I'm reverting this for now, and if someone is packaging v2025.01-rc1 too, just do that too. Sorry.
-- Tom
participants (3)
-
Moritz Fischer -
Patrick Rudolph -
Tom Rini