From 53cb97c093061b81ab3247c432d15be011353470 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:08 +0000 Subject: [PATCH 1/8] doc: Correct position of gdb '--args' parameter The '--args' parameter to gdb comes before the binary that the debugger will be attached to rather than after the binary and before the arguments. Fix that in the docs. Signed-off-by: Andrew Scull Cc: Simon Glass Reviewed-by: Simon Glass --- doc/develop/tests_sandbox.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst index 84608dcb84..40cf8ecdd7 100644 --- a/doc/develop/tests_sandbox.rst +++ b/doc/develop/tests_sandbox.rst @@ -103,7 +103,7 @@ running with -D will produce different results. You can easily use gdb on these tests, without needing --gdbserver:: - $ gdb u-boot --args -T -c "ut dm gpio" + $ gdb --args u-boot -T -c "ut dm gpio" ... (gdb) break dm_test_gpio Breakpoint 1 at 0x1415bd: file test/dm/gpio.c, line 37. From 3849ca7b2f8363dee751c6918df0aacf64cde3bd Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:09 +0000 Subject: [PATCH 2/8] acpi: Fix buffer overflow in do_acpi_dump() When do_acpi_dump() converts the table name to upper case, pass the actual size of the output buffer so that the null terminator doesn't get written beyond the end of the buffer. Signed-off-by: Andrew Scull Cc: Simon Glass Cc: Wolfgang Wallner Cc: Bin Meng Reviewed-by: Simon Glass --- cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/acpi.c b/cmd/acpi.c index c543f1e3c2..0e473b415d 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -178,7 +178,7 @@ static int do_acpi_dump(struct cmd_tbl *cmdtp, int flag, int argc, printf("Table name '%s' must be four characters\n", name); return CMD_RET_FAILURE; } - str_to_upper(name, sig, -1); + str_to_upper(name, sig, ACPI_NAME_LEN); ret = dump_table_name(sig); if (ret) { printf("Table '%.*s' not found\n", ACPI_NAME_LEN, sig); From 9c2f5ecd43ee8bad9d52497c154088c6a99e3f9d Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:10 +0000 Subject: [PATCH 3/8] x86: sandbox: Add missing PCI bar to barinfo There are expecte to be bars 0 through 5, but the last of these was missing leading to an read beyond the buffer. Add the missing element with zero values. Signed-off-by: Andrew Scull Cc: Simon Glass Cc: Bin Meng Reviewed-by: Simon Glass --- drivers/power/acpi_pmc/pmc_emul.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/power/acpi_pmc/pmc_emul.c b/drivers/power/acpi_pmc/pmc_emul.c index a61eb5bd85..8015031da8 100644 --- a/drivers/power/acpi_pmc/pmc_emul.c +++ b/drivers/power/acpi_pmc/pmc_emul.c @@ -37,6 +37,7 @@ static struct pci_bar { { 0, 0 }, { 0, 0 }, { PCI_BASE_ADDRESS_SPACE_IO, 256 }, + { 0, 0 }, }; struct pmc_emul_priv { From 62120155b67313509b673e051155075383a8a33a Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:11 +0000 Subject: [PATCH 4/8] usb: sandbox: Check for string end in copy_to_unicode() When copying the string in copy_to_unicode(), check for the null terminator in each position, not just at the start, to avoid reading beyond the end of the string. Signed-off-by: Andrew Scull Cc: Simon Glass Cc: Marek Vasut Reviewed-by: Simon Glass --- drivers/usb/emul/usb-emul-uclass.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/emul/usb-emul-uclass.c b/drivers/usb/emul/usb-emul-uclass.c index 05f6d3d9e2..b31dc950e3 100644 --- a/drivers/usb/emul/usb-emul-uclass.c +++ b/drivers/usb/emul/usb-emul-uclass.c @@ -15,13 +15,12 @@ static int copy_to_unicode(char *buff, int length, const char *str) { int ptr; - int i; if (length < 2) return 0; buff[1] = USB_DT_STRING; - for (ptr = 2, i = 0; ptr + 1 < length && *str; i++, ptr += 2) { - buff[ptr] = str[i]; + for (ptr = 2; ptr + 1 < length && *str; str++, ptr += 2) { + buff[ptr] = *str; buff[ptr + 1] = 0; } buff[0] = ptr; From beb341ae7f43a4424ca321315a25fe9133030de2 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:12 +0000 Subject: [PATCH 5/8] usb: sandbox: Bounds check read from buffer The buffer is 512 bytes but read requests can be 800 bytes. Limit the request to the size of the buffer. Signed-off-by: Andrew Scull Cc: Simon Glass Cc: Marek Vasut Reviewed-by: Simon Glass --- drivers/usb/emul/sandbox_flash.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/emul/sandbox_flash.c b/drivers/usb/emul/sandbox_flash.c index edabc1b3a7..cc80f67133 100644 --- a/drivers/usb/emul/sandbox_flash.c +++ b/drivers/usb/emul/sandbox_flash.c @@ -345,6 +345,8 @@ static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev, } else { if (priv->alloc_len && len > priv->alloc_len) len = priv->alloc_len; + if (len > sizeof(priv->buff)) + len = sizeof(priv->buff); memcpy(buff, priv->buff, len); priv->phase = PHASE_STATUS; } From 49209da54f9580c80e96b5a33351d24d59599926 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:13 +0000 Subject: [PATCH 6/8] sound: Fix buffer overflow in square wave generation Data is written for each channel but is only tracked as having one channel written. This resulted in a buffer overflow and corruption of the allocator's metadata which caused further problems when the buffer was later freed. This could be observed with sandbox unit tests. Resolve the overflow by tracking the writes for each channel. Fixes: f987177db9 ("dm: sound: Use the correct number of channels for sound") Signed-off-by: Andrew Scull Cc: Simon Glass Reviewed-by: Simon Glass --- drivers/sound/sound.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/sound/sound.c b/drivers/sound/sound.c index b0eab23391..041dfdccfe 100644 --- a/drivers/sound/sound.c +++ b/drivers/sound/sound.c @@ -25,13 +25,11 @@ void sound_create_square_wave(uint sample_rate, unsigned short *data, int size, int i, j; for (i = 0; size && i < half; i++) { - size -= 2; - for (j = 0; j < channels; j++) + for (j = 0; size && j < channels; j++, size -= 2) *data++ = amplitude; } for (i = 0; size && i < period - half; i++) { - size -= 2; - for (j = 0; j < channels; j++) + for (j = 0; size && j < channels; j++, size -= 2) *data++ = -amplitude; } } From 7f58feae3fba07c349fe57e86ce8bd5cc2198f58 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:14 +0000 Subject: [PATCH 7/8] test: Fix pointer overrun in dm_test_devm_regmap() This tests calls regmap_read() which takes a uint pointer as an output parameter. The test was passing a pointer to a u16 which resulted in an overflow when the output was written. Fix this by following the regmap_read() API and passing a uint pointer instead. Signed-off-by: Andrew Scull Cc: Simon Glass Cc: Heinrich Schuchardt Cc: Jean-Jacques Hiblot Cc: Pratyush Yadav Reviewed-by: Simon Glass --- test/dm/regmap.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/dm/regmap.c b/test/dm/regmap.c index 04bb1645d1..8560f2afc2 100644 --- a/test/dm/regmap.c +++ b/test/dm/regmap.c @@ -286,8 +286,7 @@ U_BOOT_DRIVER(regmap_test) = { static int dm_test_devm_regmap(struct unit_test_state *uts) { int i = 0; - u16 val; - void *valp = &val; + uint val; u16 pattern[REGMAP_TEST_BUF_SZ]; u16 *buffer; struct udevice *dev; @@ -311,7 +310,7 @@ static int dm_test_devm_regmap(struct unit_test_state *uts) ut_assertok(regmap_write(priv->cfg_regmap, i, pattern[i])); } for (i = 0; i < REGMAP_TEST_BUF_SZ; i++) { - ut_assertok(regmap_read(priv->cfg_regmap, i, valp)); + ut_assertok(regmap_read(priv->cfg_regmap, i, &val)); ut_asserteq(val, buffer[i]); ut_asserteq(val, pattern[i]); } @@ -319,9 +318,9 @@ static int dm_test_devm_regmap(struct unit_test_state *uts) ut_asserteq(-ERANGE, regmap_write(priv->cfg_regmap, REGMAP_TEST_BUF_SZ, val)); ut_asserteq(-ERANGE, regmap_read(priv->cfg_regmap, REGMAP_TEST_BUF_SZ, - valp)); + &val)); ut_asserteq(-ERANGE, regmap_write(priv->cfg_regmap, -1, val)); - ut_asserteq(-ERANGE, regmap_read(priv->cfg_regmap, -1, valp)); + ut_asserteq(-ERANGE, regmap_read(priv->cfg_regmap, -1, &val)); return 0; } From d69616e529560ace8cdf40bda91464a88c7ff43a Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Sun, 3 Apr 2022 10:39:15 +0000 Subject: [PATCH 8/8] test: dm: devres: Remove use-after-free Use-after-free shouldn't be used, even in tests. It's bad practice and makes the test brittle. Signed-off-by: Andrew Scull Cc: Simon Glass Reviewed-by: Simon Glass --- test/dm/devres.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/dm/devres.c b/test/dm/devres.c index 4f959d11da..524114c833 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -178,11 +178,8 @@ static int dm_test_devres_phase(struct unit_test_state *uts) ut_asserteq(1, stats.allocs); ut_asserteq(TEST_DEVRES_SIZE, stats.total_size); - /* Unbinding removes the other. Note this access a freed pointer */ + /* Unbinding removes the other. */ device_unbind(dev); - devres_get_stats(dev, &stats); - ut_asserteq(0, stats.allocs); - ut_asserteq(0, stats.total_size); return 0; }