[BUG] sandbox error handling broken on origin/next

[Heinrich Schuchardt]

Am 23. März 2021 01:57:00 MEZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Heinrich,
>
>On Tue, 23 Mar 2021 at 08:45, Heinrich Schuchardt <xypron.glpk at gmx.de>
>wrote:
>>
>> On 3/22/21 7:16 PM, Simon Glass wrote:
>> > Hi Heinrich,
>> >
>> > On Tue, 23 Mar 2021 at 07:12, Heinrich Schuchardt
><xypron.glpk at gmx.de> wrote:
>> >>
>> >> Hello Simon,
>> >>
>> >> using sandbox_defconfig on origin/master:
>> >>
>> >> Hit any key to stop autoboot:  0
>> >> => exception sigsegv
>> >>
>> >> Segmentation violation
>> >> pc = 0x55d3566d04f9, pc_reloc = 0x554f9
>> >>
>> >> $
>> >>
>> >> Here the SIGSEGV is correctly handled by the sandbox.
>> >>
>> >> On origin/next:
>> >>
>> >> => exception sigsegv
>> >>
>> >> Segmentation violation
>> >> pc = 0x5567966da96b, pc_reloc = 0x5567866da96b
>> >>
>> >> Writing sandbox state
>> >> Segmentation fault
>> >> $
>> >>
>> >> The same problem is visible when executing the poweroff command.
>> >>
>> >> => poweroff
>> >> poweroff ...
>> >> Segmentation fault
>> >> $
>> >>
>> >> Bisecting points to your commit
>> >>
>> >> b308d9fd18fa
>> >> sandbox: Avoid using malloc() for system state
>> >>
>> >> The segmentation fault occurs when os_exit() calls dm_uninit().
>> >> The value of gd is invalid at this point.
>> >
>> > Can you please check this patch?
>> >
>> >
>http://patchwork.ozlabs.org/project/uboot/patch/20210315051124.1940496-10-sjg@chromium.org/
>> >
>> > Also, is there no test covering the above?
>> >
>> > Regards,
>> > Simon
>> >
>>
>> Hello Simon,
>>
>> We have a poweroff test but there is no detection for the
>'Segmentation
>> fault' string.
>>
>> For CONFIG_SANDBOX_CRASH_RESET=n the patch helps.
>>
>> For CONFIG_SANDBOX_CRASH_RESET=y you still get a segmentation fault
>when
>> executing 'exception sigsegv'.
>
>OK, is that caused by the same commit?

Yes

> The problem is that the commit
>is actually fixing a bug. I'll need to think about how to fix the fix.
>
>>
>> Unfortunately you decided to disable CONFIG_SANDBOX_CRASH_RESET in
>> sandbox_defconfig. Otherwise you would have detected the problem as
>> "FAILED test/py/tests/test_sandbox_exit.py::test_exception_reset".
>
>Well we don't generally want to reset when we get a crash, right?
>

Resetting after a crash is what all other boards do.

I personally don't have a need for the sandbox to behave differently.

>>
>> Please, adjust sandbox_reset().
>
>What would you like it to do?

Executing the 'reset' command fails with a crash.

The crash occurs when dm_uninit() is invoked.

Best regards

Heinrich