[PATCH v2 0/2] Console/stdio use after free

[PATCH v2 0/2] Console/stdio use after free

[Nicolas Saenz Julienne]

With today's master, 70c2525c0d3c ('IOMUX: Stop dropped consoles')
introduces a use after free in usb_kbd_remove():

- usbkbd's stdio device is de-registered with stdio_deregister_dev(),
  the struct stdio_dev is freed.

- iomux_doenv() is called, usbkbd removed from the console list, and
  console_stop() is called on the struct stdio_dev pointer that no
  longer exists.

This series mitigates this by making sure the pointer is really a stdio
device prior performing the stop operation. It's not ideal, but I
couldn't figure out a nicer way to fix this.

Regards,
Nicolas

---

Changes since v1:
 - Comment new function
 - Add comment stating this needs a proper fix

Nicolas Saenz Julienne (2):
  stdio: Introduce stdio_valid()
  console: Don't start/stop console if stdio device invalid

 common/console.c    |  9 +++++++++
 common/stdio.c      | 11 +++++++++++
 include/stdio_dev.h | 11 +++++++++++
 3 files changed, 31 insertions(+)

-- 
2.30.0

[PATCH v2 1/2] stdio: Introduce stdio_valid()

[Nicolas Saenz Julienne]

stdio_valid() will confirm that a struct stdio_dev pointer is indeed
valid.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---

Changes since v1:
 - Properly document function

 common/stdio.c      | 11 +++++++++++
 include/stdio_dev.h | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/common/stdio.c b/common/stdio.c
index abf9b1e915..69b7d2692d 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -157,6 +157,17 @@ static int stdio_probe_device(const char *name, enum uclass_id id,
 	return 0;
 }
 
+bool stdio_valid(struct stdio_dev *dev)
+{
+	struct stdio_dev *sdev;
+
+	list_for_each_entry(sdev, &devs.list, list)
+		if (sdev == dev)
+			return true;
+
+	return false;
+}
+
 struct stdio_dev *stdio_get_by_name(const char *name)
 {
 	struct list_head *pos;
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index 48871a6a22..e9d610a840 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -98,6 +98,17 @@ struct list_head *stdio_get_list(void);
 struct stdio_dev *stdio_get_by_name(const char *name);
 struct stdio_dev *stdio_clone(struct stdio_dev *dev);
 
+/**
+ * stdio_valid() - Validate stdio device exists
+ *
+ * Verifies that dev is contained in the global list of stdio devices.
+ *
+ * @dev: Stdio device pointer
+ *
+ * returns 'true' when valid, 'false' otherwise
+ */
+bool stdio_valid(struct stdio_dev *dev);
+
 int drv_lcd_init(void);
 int drv_video_init(void);
 int drv_keyboard_init(void);
-- 
2.30.0

[PATCH v2 1/2] stdio: Introduce stdio_valid()

[Simon Glass]

On Thu, 28 Jan 2021 at 06:12, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> stdio_valid() will confirm that a struct stdio_dev pointer is indeed
> valid.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> ---
>
> Changes since v1:
>  - Properly document function
>
>  common/stdio.c      | 11 +++++++++++
>  include/stdio_dev.h | 11 +++++++++++
>  2 files changed, 22 insertions(+)

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

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Nicolas Saenz Julienne]

Don't start/stop an stdio device that might have been already freed.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")

---
Changes since v1:
 - Add comment stating this should be properly fixed

 common/console.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/console.c b/common/console.c
index f3cc45cab5..570f26d149 100644
--- a/common/console.c
+++ b/common/console.c
@@ -252,6 +252,15 @@ static bool console_needs_start_stop(int file, struct stdio_dev *sdev)
 {
 	int i, j;
 
+	/*
+	 * TODO: This is a workaround to avoid accessing freed memory:
+	 * console_stop() might be called on an stdio_dev that has already been
+	 * de-registered, due to the fact that stdio_deregister_dev()
+	 * doesn't update the global console_devices array.
+	 */
+	if (!stdio_valid(sdev))
+		return false;
+
 	for (i = 0; i < ARRAY_SIZE(cd_count); i++) {
 		if (i == file)
 			continue;
-- 
2.30.0

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> Don't start/stop an stdio device that might have been already freed.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")

...

> +	/*
> +	 * TODO: This is a workaround to avoid accessing freed memory:
> +	 * console_stop() might be called on an stdio_dev that has already been
> +	 * de-registered, due to the fact that stdio_deregister_dev()
> +	 * doesn't update the global console_devices array.
> +	 */

I see now. I think I have experienced this issue from time to time. I will look
at it. Tom, Simon, please hold on applying these for a while.

> +	if (!stdio_valid(sdev))
> +		return false;

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Matthias Brugger]

On 28/01/2021 16:52, Andy Shevchenko wrote:
> On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
>> Don't start/stop an stdio device that might have been already freed.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> 
> ...
> 
>> +	/*
>> +	 * TODO: This is a workaround to avoid accessing freed memory:
>> +	 * console_stop() might be called on an stdio_dev that has already been
>> +	 * de-registered, due to the fact that stdio_deregister_dev()
>> +	 * doesn't update the global console_devices array.
>> +	 */
> 
> I see now. I think I have experienced this issue from time to time. I will look
> at it. Tom, Simon, please hold on applying these for a while.
> 

Just for the notes, the failing tests hold back Nicolas series to support
RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
a long time to fix the test environment, I'd vote to take this series as a
stop-gap so that we can support that HW in the next release.

Regards,
Matthias

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=223890

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Tom Rini]

On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
> 
> 
> On 28/01/2021 16:52, Andy Shevchenko wrote:
> > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> >> Don't start/stop an stdio device that might have been already freed.
> >>
> >> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> > 
> > ...
> > 
> >> +	/*
> >> +	 * TODO: This is a workaround to avoid accessing freed memory:
> >> +	 * console_stop() might be called on an stdio_dev that has already been
> >> +	 * de-registered, due to the fact that stdio_deregister_dev()
> >> +	 * doesn't update the global console_devices array.
> >> +	 */
> > 
> > I see now. I think I have experienced this issue from time to time. I will look
> > at it. Tom, Simon, please hold on applying these for a while.
> > 
> 
> Just for the notes, the failing tests hold back Nicolas series to support
> RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
> a long time to fix the test environment, I'd vote to take this series as a
> stop-gap so that we can support that HW in the next release.

Andy, since you're working on a better solution, do you want more time
for that or should I pick this series up for now and you can revert it
as part of your better fix?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210201/686390a0/attachment.sig>

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Andy Shevchenko]

On Mon, Feb 1, 2021 at 9:29 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
> >
> >
> > On 28/01/2021 16:52, Andy Shevchenko wrote:
> > > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> > >> Don't start/stop an stdio device that might have been already freed.
> > >>
> > >> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> > >
> > > ...
> > >
> > >> +  /*
> > >> +   * TODO: This is a workaround to avoid accessing freed memory:
> > >> +   * console_stop() might be called on an stdio_dev that has already been
> > >> +   * de-registered, due to the fact that stdio_deregister_dev()
> > >> +   * doesn't update the global console_devices array.
> > >> +   */
> > >
> > > I see now. I think I have experienced this issue from time to time. I will look
> > > at it. Tom, Simon, please hold on applying these for a while.
> > >
> >
> > Just for the notes, the failing tests hold back Nicolas series to support
> > RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
> > a long time to fix the test environment, I'd vote to take this series as a
> > stop-gap so that we can support that HW in the next release.
>
> Andy, since you're working on a better solution, do you want more time
> for that or should I pick this series up for now and you can revert it
> as part of your better fix?  Thanks!

Sorry for the delayed reply. Give me a couple of days, and if I will
come up without any (good) solution, apply this series.


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Andy Shevchenko]

On Wed, Feb 03, 2021 at 11:50:53AM +0200, Andy Shevchenko wrote:
> On Mon, Feb 1, 2021 at 9:29 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
> > >
> > >
> > > On 28/01/2021 16:52, Andy Shevchenko wrote:
> > > > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> > > >> Don't start/stop an stdio device that might have been already freed.
> > > >>
> > > >> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> > > >
> > > > ...
> > > >
> > > >> +  /*
> > > >> +   * TODO: This is a workaround to avoid accessing freed memory:
> > > >> +   * console_stop() might be called on an stdio_dev that has already been
> > > >> +   * de-registered, due to the fact that stdio_deregister_dev()
> > > >> +   * doesn't update the global console_devices array.
> > > >> +   */
> > > >
> > > > I see now. I think I have experienced this issue from time to time. I will look
> > > > at it. Tom, Simon, please hold on applying these for a while.
> > > >
> > >
> > > Just for the notes, the failing tests hold back Nicolas series to support
> > > RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
> > > a long time to fix the test environment, I'd vote to take this series as a
> > > stop-gap so that we can support that HW in the next release.
> >
> > Andy, since you're working on a better solution, do you want more time
> > for that or should I pick this series up for now and you can revert it
> > as part of your better fix?  Thanks!
> 
> Sorry for the delayed reply. Give me a couple of days, and if I will
> come up without any (good) solution, apply this series.

Okay, as promised, I prepared a branch [1] with new approach, but while I will
be busy with other stuff, I would like you to test on real hardware and tell if
it helps. At least it passes test cases.

If it works, I would like to get a Tested-by tag and will prepare and submit
the formal series.

[1]: https://github.com/andy-shev/u-boot/tree/iomux

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Tom Rini]

On Fri, Feb 05, 2021 at 07:06:54PM +0200, Andy Shevchenko wrote:
> On Wed, Feb 03, 2021 at 11:50:53AM +0200, Andy Shevchenko wrote:
> > On Mon, Feb 1, 2021 at 9:29 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
> > > >
> > > >
> > > > On 28/01/2021 16:52, Andy Shevchenko wrote:
> > > > > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> > > > >> Don't start/stop an stdio device that might have been already freed.
> > > > >>
> > > > >> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> > > > >
> > > > > ...
> > > > >
> > > > >> +  /*
> > > > >> +   * TODO: This is a workaround to avoid accessing freed memory:
> > > > >> +   * console_stop() might be called on an stdio_dev that has already been
> > > > >> +   * de-registered, due to the fact that stdio_deregister_dev()
> > > > >> +   * doesn't update the global console_devices array.
> > > > >> +   */
> > > > >
> > > > > I see now. I think I have experienced this issue from time to time. I will look
> > > > > at it. Tom, Simon, please hold on applying these for a while.
> > > > >
> > > >
> > > > Just for the notes, the failing tests hold back Nicolas series to support
> > > > RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
> > > > a long time to fix the test environment, I'd vote to take this series as a
> > > > stop-gap so that we can support that HW in the next release.
> > >
> > > Andy, since you're working on a better solution, do you want more time
> > > for that or should I pick this series up for now and you can revert it
> > > as part of your better fix?  Thanks!
> > 
> > Sorry for the delayed reply. Give me a couple of days, and if I will
> > come up without any (good) solution, apply this series.
> 
> Okay, as promised, I prepared a branch [1] with new approach, but while I will
> be busy with other stuff, I would like you to test on real hardware and tell if
> it helps. At least it passes test cases.
> 
> If it works, I would like to get a Tested-by tag and will prepare and submit
> the formal series.
> 
> [1]: https://github.com/andy-shev/u-boot/tree/iomux

I reliably get:

========================================== FAILURES ===========================================
_____________________________ test_ut[ut_dm_fdt_livetree_writing] _____________________________

u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object@0x7fe251aeca90>
ut_subtest = 'dm fdt_livetree_writing'

    def test_ut(u_boot_console, ut_subtest):
        """Execute a "ut" subtest.

        The subtests are collected in function generate_ut_subtest() from linker
        generated lists by applying a regular expression to the lines of file
        u-boot.sym. The list entries are created using the C macro UNIT_TEST().

        Strict naming conventions have to be followed to match the regular
        expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in
        test suite foo that can be executed via command 'ut foo bar' and is
        implemented in C function foo_test_bar().

        Args:
            u_boot_console (ConsoleBase): U-Boot console
            ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to
                execute command 'ut foo bar'
        """

        output = u_boot_console.run_command('ut ' + ut_subtest)
>       assert output.endswith('Failures: 0')
E       AssertionError: assert False
E        +  where False = <built-in method endswith of str object@0x7fe2516c5da0>('Failures: 0')
E        +    where <built-in method endswith of str object@0x7fe2516c5da0> = 'Test: dm_test_fdt_livetree_writing: test-fdt.c\r\r\ntest/dm/test-fdt.c:837, dm_test_fdt_livetree_writing(): FDT_ADDR_...\r\nTest: dm_test_fdt_livetree_writing: test-fdt.c (flat tree)\r\r\nLive tree not active; ignore test\r\r\nFailures: 1'.endswith

test/py/tests/test_ut.py:43: AssertionError
------------------------------------ Captured stdout call -------------------------------------
=> ut dm fdt_livetree_writing
Test: dm_test_fdt_livetree_writing: test-fdt.c
test/dm/test-fdt.c:837, dm_test_fdt_livetree_writing(): FDT_ADDR_T_NONE == dev_read_addr(dev): Expected 0xffffffff (-1), got 0x42 (66)
Test: dm_test_fdt_livetree_writing: test-fdt.c (flat tree)
Live tree not active; ignore test
Failures: 1
=>

On sandbox.  I'm going to take sandbox out of my testing loop for the
moment and see what Pi and a few others do.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210205/5e1cc7a1/attachment.sig>

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Tom Rini]

On Fri, Feb 05, 2021 at 12:50:56PM -0500, Tom Rini wrote:
> On Fri, Feb 05, 2021 at 07:06:54PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 03, 2021 at 11:50:53AM +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 1, 2021 at 9:29 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Jan 29, 2021 at 09:50:50AM +0100, Matthias Brugger wrote:
> > > > >
> > > > >
> > > > > On 28/01/2021 16:52, Andy Shevchenko wrote:
> > > > > > On Thu, Jan 28, 2021 at 02:12:40PM +0100, Nicolas Saenz Julienne wrote:
> > > > > >> Don't start/stop an stdio device that might have been already freed.
> > > > > >>
> > > > > >> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > > >> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
> > > > > >
> > > > > > ...
> > > > > >
> > > > > >> +  /*
> > > > > >> +   * TODO: This is a workaround to avoid accessing freed memory:
> > > > > >> +   * console_stop() might be called on an stdio_dev that has already been
> > > > > >> +   * de-registered, due to the fact that stdio_deregister_dev()
> > > > > >> +   * doesn't update the global console_devices array.
> > > > > >> +   */
> > > > > >
> > > > > > I see now. I think I have experienced this issue from time to time. I will look
> > > > > > at it. Tom, Simon, please hold on applying these for a while.
> > > > > >
> > > > >
> > > > > Just for the notes, the failing tests hold back Nicolas series to support
> > > > > RPi400/CM4 [1] as it does not run the new tests added successfully. If it takes
> > > > > a long time to fix the test environment, I'd vote to take this series as a
> > > > > stop-gap so that we can support that HW in the next release.
> > > >
> > > > Andy, since you're working on a better solution, do you want more time
> > > > for that or should I pick this series up for now and you can revert it
> > > > as part of your better fix?  Thanks!
> > > 
> > > Sorry for the delayed reply. Give me a couple of days, and if I will
> > > come up without any (good) solution, apply this series.
> > 
> > Okay, as promised, I prepared a branch [1] with new approach, but while I will
> > be busy with other stuff, I would like you to test on real hardware and tell if
> > it helps. At least it passes test cases.
> > 
> > If it works, I would like to get a Tested-by tag and will prepare and submit
> > the formal series.
> > 
> > [1]: https://github.com/andy-shev/u-boot/tree/iomux
> 
> I reliably get:
> 
> ========================================== FAILURES ===========================================
> _____________________________ test_ut[ut_dm_fdt_livetree_writing] _____________________________
> 
> u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object@0x7fe251aeca90>
> ut_subtest = 'dm fdt_livetree_writing'
> 
>     def test_ut(u_boot_console, ut_subtest):
>         """Execute a "ut" subtest.
> 
>         The subtests are collected in function generate_ut_subtest() from linker
>         generated lists by applying a regular expression to the lines of file
>         u-boot.sym. The list entries are created using the C macro UNIT_TEST().
> 
>         Strict naming conventions have to be followed to match the regular
>         expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in
>         test suite foo that can be executed via command 'ut foo bar' and is
>         implemented in C function foo_test_bar().
> 
>         Args:
>             u_boot_console (ConsoleBase): U-Boot console
>             ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to
>                 execute command 'ut foo bar'
>         """
> 
>         output = u_boot_console.run_command('ut ' + ut_subtest)
> >       assert output.endswith('Failures: 0')
> E       AssertionError: assert False
> E        +  where False = <built-in method endswith of str object@0x7fe2516c5da0>('Failures: 0')
> E        +    where <built-in method endswith of str object@0x7fe2516c5da0> = 'Test: dm_test_fdt_livetree_writing: test-fdt.c\r\r\ntest/dm/test-fdt.c:837, dm_test_fdt_livetree_writing(): FDT_ADDR_...\r\nTest: dm_test_fdt_livetree_writing: test-fdt.c (flat tree)\r\r\nLive tree not active; ignore test\r\r\nFailures: 1'.endswith
> 
> test/py/tests/test_ut.py:43: AssertionError
> ------------------------------------ Captured stdout call -------------------------------------
> => ut dm fdt_livetree_writing
> Test: dm_test_fdt_livetree_writing: test-fdt.c
> test/dm/test-fdt.c:837, dm_test_fdt_livetree_writing(): FDT_ADDR_T_NONE == dev_read_addr(dev): Expected 0xffffffff (-1), got 0x42 (66)
> Test: dm_test_fdt_livetree_writing: test-fdt.c (flat tree)
> Live tree not active; ignore test
> Failures: 1
> =>
> 
> On sandbox.  I'm going to take sandbox out of my testing loop for the
> moment and see what Pi and a few others do.

As expected, the rest of the tests (which did pass in sandbox) also pass
on Pi 3 and a few other platforms I have here.  Didn't test USB, etc,
etc, just pytest.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210205/9c61aa40/attachment.sig>

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Andy Shevchenko]

On Fri, Feb 05, 2021 at 01:07:02PM -0500, Tom Rini wrote:
> On Fri, Feb 05, 2021 at 12:50:56PM -0500, Tom Rini wrote:
> > On Fri, Feb 05, 2021 at 07:06:54PM +0200, Andy Shevchenko wrote:
> > > On Wed, Feb 03, 2021 at 11:50:53AM +0200, Andy Shevchenko wrote:

...

> > > Okay, as promised, I prepared a branch [1] with new approach, but while I will
> > > be busy with other stuff, I would like you to test on real hardware and tell if
> > > it helps. At least it passes test cases.
> > > 
> > > If it works, I would like to get a Tested-by tag and will prepare and submit
> > > the formal series.
> > > 
> > > [1]: https://github.com/andy-shev/u-boot/tree/iomux
> > 
> > I reliably get:

It's one of the my patches revealed this, I reverted for now. Please, retest.


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Tom Rini]

On Fri, Feb 05, 2021 at 09:31:00PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 05, 2021 at 01:07:02PM -0500, Tom Rini wrote:
> > On Fri, Feb 05, 2021 at 12:50:56PM -0500, Tom Rini wrote:
> > > On Fri, Feb 05, 2021 at 07:06:54PM +0200, Andy Shevchenko wrote:
> > > > On Wed, Feb 03, 2021 at 11:50:53AM +0200, Andy Shevchenko wrote:
> 
> ...
> 
> > > > Okay, as promised, I prepared a branch [1] with new approach, but while I will
> > > > be busy with other stuff, I would like you to test on real hardware and tell if
> > > > it helps. At least it passes test cases.
> > > > 
> > > > If it works, I would like to get a Tested-by tag and will prepare and submit
> > > > the formal series.
> > > > 
> > > > [1]: https://github.com/andy-shev/u-boot/tree/iomux
> > > 
> > > I reliably get:
> 
> It's one of the my patches revealed this, I reverted for now. Please, retest.

pytest now passes, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210205/b5aff639/attachment.sig>

[PATCH v2 2/2] console: Don't start/stop console if stdio device invalid

[Simon Glass]

Hi Nicolas,

On Thu, 28 Jan 2021 at 06:12, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Don't start/stop an stdio device that might have been already freed.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Fixes: 70c2525c0d3c ("IOMUX: Stop dropped consoles")
>
> ---
> Changes since v1:
>  - Add comment stating this should be properly fixed
>
>  common/console.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>

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

Since this says it is a stopgap, when does the real fix come?

Regards,
Simon

[PATCH v2 0/2] Console/stdio use after free

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 02:12:38PM +0100, Nicolas Saenz Julienne wrote:
> With today's master, 70c2525c0d3c ('IOMUX: Stop dropped consoles')
> introduces a use after free in usb_kbd_remove():
> 
> - usbkbd's stdio device is de-registered with stdio_deregister_dev(),
>   the struct stdio_dev is freed.
> 
> - iomux_doenv() is called, usbkbd removed from the console list, and
>   console_stop() is called on the struct stdio_dev pointer that no
>   longer exists.
> 
> This series mitigates this by making sure the pointer is really a stdio
> device prior performing the stop operation. It's not ideal, but I
> couldn't figure out a nicer way to fix this.

I have just sent another approach, can you test it instead, please?

-- 
With Best Regards,
Andy Shevchenko