[BUG] sandbox error handling broken on origin/next

[Simon Glass]

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? 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?

>
> Please, adjust sandbox_reset().

What would you like it to do?

Regards,
Simon