[PATCH 0/8] Fix misc ASAN reports

[PATCH 0/8] Fix misc ASAN reports

[Andrew Scull]

I've been experimenting with ASAN on sandbox and turned up a few issues
that are fixed in this series.

Basic ASAN was easy to turn on, but integrating with dlmalloc was
messier and fairly intrusive. Even when I had it working, there was only
a small redzone between allocations which limits the usefulness.

I saw another series on the list by Sean Anderson to enable valgrind
which was finding a different set of issues, though there was one
overlap that Sean is fixing with
"[PATCH] IOMUX: Fix access past end of console_devices".

With these issues fixed, I was able to run the dm tests without any ASAN
issues. There are a couple of leaks reported at the end, but that's for
another day.

Andrew Scull (8):
  doc: Correct position of gdb '--args' parameter
  acpi: Fix buffer overflow in do_acpi_dump()
  x86: sandbox: Add missing PCI bar to barinfo
  usb: sandbox: Check for string end in copy_to_unicode()
  usb: sandbox: Bounds check read from buffer
  sound: Fix buffer overflow in square wave generation
  test: Fix pointer overrun in dm_test_devm_regmap()
  test: dm: devres: Remove use-after-free

 cmd/acpi.c                         | 2 +-
 doc/develop/tests_sandbox.rst      | 2 +-
 drivers/power/acpi_pmc/pmc_emul.c  | 1 +
 drivers/sound/sound.c              | 6 ++----
 drivers/usb/emul/sandbox_flash.c   | 2 ++
 drivers/usb/emul/usb-emul-uclass.c | 5 ++---
 test/dm/devres.c                   | 5 +----
 test/dm/regmap.c                   | 9 ++++-----
 8 files changed, 14 insertions(+), 18 deletions(-)

-- 
2.35.1.1094.g7c7d902a7c-goog

[PATCH 1/8] doc: Correct position of gdb '--args' parameter

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
---
 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.
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 1/8] doc: Correct position of gdb '--args' parameter

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  doc/develop/tests_sandbox.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 1/8] doc: Correct position of gdb '--args' parameter

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Sun, Apr 03, 2022 at 10:39:08AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 2/8] acpi: Fix buffer overflow in do_acpi_dump()

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
---
 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);
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 2/8] acpi: Fix buffer overflow in do_acpi_dump()

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>  cmd/acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 2/8] acpi: Fix buffer overflow in do_acpi_dump()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

On Sun, Apr 03, 2022 at 10:39:09AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 3/8] x86: sandbox: Add missing PCI bar to barinfo

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
---
 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 {
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 3/8] x86: sandbox: Add missing PCI bar to barinfo

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/power/acpi_pmc/pmc_emul.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 3/8] x86: sandbox: Add missing PCI bar to barinfo

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On Sun, Apr 03, 2022 at 10:39:10AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 4/8] usb: sandbox: Check for string end in copy_to_unicode()

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Marek Vasut <marex@denx.de>
---
 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;
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 4/8] usb: sandbox: Check for string end in copy_to_unicode()

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  drivers/usb/emul/usb-emul-uclass.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 4/8] usb: sandbox: Check for string end in copy_to_unicode()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On Sun, Apr 03, 2022 at 10:39:11AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 5/8] usb: sandbox: Bounds check read from buffer

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Marek Vasut <marex@denx.de>
---
 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;
 			}
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 5/8] usb: sandbox: Bounds check read from buffer

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  drivers/usb/emul/sandbox_flash.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 5/8] usb: sandbox: Bounds check read from buffer

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Sun, Apr 03, 2022 at 10:39:12AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 6/8] sound: Fix buffer overflow in square wave generation

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
---
 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;
 		}
 	}
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 6/8] sound: Fix buffer overflow in square wave generation

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/sound/sound.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 6/8] sound: Fix buffer overflow in square wave generation

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

On Sun, Apr 03, 2022 at 10:39:13AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 7/8] test: Fix pointer overrun in dm_test_devm_regmap()

[Andrew Scull]

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 <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: Pratyush Yadav <p.yadav@ti.com>
---
 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;
 }
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 7/8] test: Fix pointer overrun in dm_test_devm_regmap()

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Pratyush Yadav <p.yadav@ti.com>
> ---
>  test/dm/regmap.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 7/8] test: Fix pointer overrun in dm_test_devm_regmap()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Sun, Apr 03, 2022 at 10:39:14AM +0000, Andrew Scull wrote:

> 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 <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Pratyush Yadav <p.yadav@ti.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 8/8] test: dm: devres: Remove use-after-free

[Andrew Scull]

Use-after-free shouldn't be used, even in tests. It's bad practice and
makes the test brittle.

Signed-off-by: Andrew Scull <ascull@google.com>
Cc: Simon Glass <sjg@chromium.org>
---
 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;
 }
-- 
2.35.1.1094.g7c7d902a7c-goog

Re: [PATCH 8/8] test: dm: devres: Remove use-after-free

[Simon Glass]

On Sun, 3 Apr 2022 at 04:39, Andrew Scull <ascull@google.com> wrote:
>
> Use-after-free shouldn't be used, even in tests. It's bad practice and
> makes the test brittle.
>
> Signed-off-by: Andrew Scull <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  test/dm/devres.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 8/8] test: dm: devres: Remove use-after-free

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

On Sun, Apr 03, 2022 at 10:39:15AM +0000, Andrew Scull wrote:

> Use-after-free shouldn't be used, even in tests. It's bad practice and
> makes the test brittle.
> 
> Signed-off-by: Andrew Scull <ascull@google.com>
> Cc: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 0/8] Fix misc ASAN reports

[Sean Anderson]

On 4/3/22 6:39 AM, Andrew Scull wrote:
> I've been experimenting with ASAN on sandbox and turned up a few issues
> that are fixed in this series.
> 
> Basic ASAN was easy to turn on, but integrating with dlmalloc was
> messier and fairly intrusive. Even when I had it working, there was only
> a small redzone between allocations which limits the usefulness.

Do you have any patches for this?

--Sean

> I saw another series on the list by Sean Anderson to enable valgrind
> which was finding a different set of issues, though there was one
> overlap that Sean is fixing with
> "[PATCH] IOMUX: Fix access past end of console_devices".
> 
> With these issues fixed, I was able to run the dm tests without any ASAN
> issues. There are a couple of leaks reported at the end, but that's for
> another day.
> 
> Andrew Scull (8):
>    doc: Correct position of gdb '--args' parameter
>    acpi: Fix buffer overflow in do_acpi_dump()
>    x86: sandbox: Add missing PCI bar to barinfo
>    usb: sandbox: Check for string end in copy_to_unicode()
>    usb: sandbox: Bounds check read from buffer
>    sound: Fix buffer overflow in square wave generation
>    test: Fix pointer overrun in dm_test_devm_regmap()
>    test: dm: devres: Remove use-after-free
> 
>   cmd/acpi.c                         | 2 +-
>   doc/develop/tests_sandbox.rst      | 2 +-
>   drivers/power/acpi_pmc/pmc_emul.c  | 1 +
>   drivers/sound/sound.c              | 6 ++----
>   drivers/usb/emul/sandbox_flash.c   | 2 ++
>   drivers/usb/emul/usb-emul-uclass.c | 5 ++---
>   test/dm/devres.c                   | 5 +----
>   test/dm/regmap.c                   | 9 ++++-----
>   8 files changed, 14 insertions(+), 18 deletions(-)
> 

Re: [PATCH 0/8] Fix misc ASAN reports

[Andrew Scull]

On Wed, 6 Apr 2022 at 19:31, Sean Anderson <seanga2@gmail.com> wrote:
>
> On 4/3/22 6:39 AM, Andrew Scull wrote:
> > I've been experimenting with ASAN on sandbox and turned up a few issues
> > that are fixed in this series.
> >
> > Basic ASAN was easy to turn on, but integrating with dlmalloc was
> > messier and fairly intrusive. Even when I had it working, there was only
> > a small redzone between allocations which limits the usefulness.
>
> Do you have any patches for this?

They're becoming less of a mess as we speak, except the dlmalloc part
which I don't have a good idea how to make neater. I'm actually doing
it as a prerequisite for fuzzing, which I've almost got an acceptable
feeling framework for and hope to be able to share fairly soon. But I
could post just the ASAN parts separately if there's interest for
that.

If you're using gcc, you can just add -fsanitize=address to the
compiler and linker commands; clang requires some more work to rename
symbols that begin with '.'.

>
> --Sean
>
> > I saw another series on the list by Sean Anderson to enable valgrind
> > which was finding a different set of issues, though there was one
> > overlap that Sean is fixing with
> > "[PATCH] IOMUX: Fix access past end of console_devices".
> >
> > With these issues fixed, I was able to run the dm tests without any ASAN
> > issues. There are a couple of leaks reported at the end, but that's for
> > another day.
> >
> > Andrew Scull (8):
> >    doc: Correct position of gdb '--args' parameter
> >    acpi: Fix buffer overflow in do_acpi_dump()
> >    x86: sandbox: Add missing PCI bar to barinfo
> >    usb: sandbox: Check for string end in copy_to_unicode()
> >    usb: sandbox: Bounds check read from buffer
> >    sound: Fix buffer overflow in square wave generation
> >    test: Fix pointer overrun in dm_test_devm_regmap()
> >    test: dm: devres: Remove use-after-free
> >
> >   cmd/acpi.c                         | 2 +-
> >   doc/develop/tests_sandbox.rst      | 2 +-
> >   drivers/power/acpi_pmc/pmc_emul.c  | 1 +
> >   drivers/sound/sound.c              | 6 ++----
> >   drivers/usb/emul/sandbox_flash.c   | 2 ++
> >   drivers/usb/emul/usb-emul-uclass.c | 5 ++---
> >   test/dm/devres.c                   | 5 +----
> >   test/dm/regmap.c                   | 9 ++++-----
> >   8 files changed, 14 insertions(+), 18 deletions(-)
> >
>