All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] sandbox error handling broken on origin/next
       [not found] <06f27a8d-11f7-24f7-760e-afcfacd3b896@gmx.de>
@ 2021-03-22 18:16 ` Simon Glass
  2021-03-22 19:45   ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2021-03-22 18:16 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, 23 Mar 2021 at 07:12, Heinrich Schuchardt <xypron.glpk@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 at chromium.org/

Also, is there no test covering the above?

Regards,
Simon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [BUG] sandbox error handling broken on origin/next
  2021-03-22 18:16 ` [BUG] sandbox error handling broken on origin/next Simon Glass
@ 2021-03-22 19:45   ` Heinrich Schuchardt
  2021-03-23  0:57     ` Simon Glass
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2021-03-22 19:45 UTC (permalink / raw)
  To: u-boot

On 3/22/21 7:16 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Tue, 23 Mar 2021 at 07:12, Heinrich Schuchardt <xypron.glpk@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 at 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'.

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".

Please, adjust sandbox_reset().

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [BUG] sandbox error handling broken on origin/next
  2021-03-22 19:45   ` Heinrich Schuchardt
@ 2021-03-23  0:57     ` Simon Glass
  2021-03-23  1:22       ` Heinrich Schuchardt
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2021-03-23  0:57 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, 23 Mar 2021 at 08:45, Heinrich Schuchardt <xypron.glpk@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@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 at 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [BUG] sandbox error handling broken on origin/next
  2021-03-23  0:57     ` Simon Glass
@ 2021-03-23  1:22       ` Heinrich Schuchardt
  2021-04-21  7:15         ` Simon Glass
  0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2021-03-23  1:22 UTC (permalink / raw)
  To: u-boot

Am 23. M?rz 2021 01:57:00 MEZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Tue, 23 Mar 2021 at 08:45, Heinrich Schuchardt <xypron.glpk@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@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 at 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [BUG] sandbox error handling broken on origin/next
  2021-03-23  1:22       ` Heinrich Schuchardt
@ 2021-04-21  7:15         ` Simon Glass
  0 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2021-04-21  7:15 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, 23 Mar 2021 at 14:22, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 23. M?rz 2021 01:57:00 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Tue, 23 Mar 2021 at 08:45, Heinrich Schuchardt <xypron.glpk@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@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 at 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.

I see sandbox as a normal program. If you run grimp and it crashes, it
doesn't reset. The only reason we have a reset feature in sandbox is
for testing purposes.

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

This is definitely a tricky area. Since devices are allocated in the
'emulated' RAM we really can't close down sandbox before uniniting
devices, and vice versa. Some separation is needed.

I will take a look at this at some point, but it needs a fair bit of thought.

Regards,
Simon

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-21  7:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <06f27a8d-11f7-24f7-760e-afcfacd3b896@gmx.de>
2021-03-22 18:16 ` [BUG] sandbox error handling broken on origin/next Simon Glass
2021-03-22 19:45   ` Heinrich Schuchardt
2021-03-23  0:57     ` Simon Glass
2021-03-23  1:22       ` Heinrich Schuchardt
2021-04-21  7:15         ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.