| Message ID | 20210705174300.18792-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | fd25ca3275946476d5c3fa32e3e7e3087fa5c572 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [1/1] sandbox: don't set SA_NODEFER in signal handler | expand |
Hi Heinrich, On Mon, 5 Jul 2021 at 11:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > The sandbox can handle signals. Due to a damaged global data pointer > additional exceptions in the signal handler may occur leading to an endless > loop. In this case leave the handling of the secondary exception to the > operating system. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > arch/sandbox/cpu/os.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I'm OK with this since it doesn't actually matter, now that the signal-catching behaviour is optional. But as I understand it, it will mean that the second exception is reported, but not the first? In other words, any core dump will be 'incorrect' in that it is not suitable for postmortem debugging. > > diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c > index 0d21827e1b..9a73ae3b18 100644 > --- a/arch/sandbox/cpu/os.c > +++ b/arch/sandbox/cpu/os.c > @@ -226,7 +226,7 @@ int os_setup_signal_handlers(void) > > act.sa_sigaction = os_signal_handler; > sigemptyset(&act.sa_mask); > - act.sa_flags = SA_SIGINFO | SA_NODEFER; > + act.sa_flags = SA_SIGINFO; > if (sigaction(SIGILL, &act, NULL) || > sigaction(SIGBUS, &act, NULL) || > sigaction(SIGSEGV, &act, NULL)) > -- > 2.30.2 > Regards, Simon
Hi Heinrich, On Wed, 7 Jul 2021 at 11:37, Simon Glass <sjg@chromium.org> wrote: > > Hi Heinrich, > > On Mon, 5 Jul 2021 at 11:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > The sandbox can handle signals. Due to a damaged global data pointer > > additional exceptions in the signal handler may occur leading to an endless > > loop. In this case leave the handling of the secondary exception to the > > operating system. > > > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > --- > > arch/sandbox/cpu/os.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I'm OK with this since it doesn't actually matter, now that the > signal-catching behaviour is optional. > > But as I understand it, it will mean that the second exception is > reported, but not the first? In other words, any core dump will be > 'incorrect' in that it is not suitable for postmortem debugging. I tested this by: - setting sandbox's 'state' pointer to NULL in the 'md' command - dropping state = &main_state from state_uninit - running it and typing 'md' This causes a recursive crash without your patch. With your patch it is not recursive. So I think it makes the -S option work better. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon
Hi Heinrich, On Wed, 7 Jul 2021 at 11:37, Simon Glass <sjg@chromium.org> wrote: > > Hi Heinrich, > > On Mon, 5 Jul 2021 at 11:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > The sandbox can handle signals. Due to a damaged global data pointer > > additional exceptions in the signal handler may occur leading to an endless > > loop. In this case leave the handling of the secondary exception to the > > operating system. > > > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > --- > > arch/sandbox/cpu/os.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I'm OK with this since it doesn't actually matter, now that the > signal-catching behaviour is optional. > > But as I understand it, it will mean that the second exception is > reported, but not the first? In other words, any core dump will be > 'incorrect' in that it is not suitable for postmortem debugging. I tested this by: - setting sandbox's 'state' pointer to NULL in the 'md' command - dropping state = &main_state from state_uninit - running it and typing 'md' This causes a recursive crash without your patch. With your patch it is not recursive. So I think it makes the -S option work better. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon Applied to u-boot-dm, thanks!
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 0d21827e1b..9a73ae3b18 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -226,7 +226,7 @@ int os_setup_signal_handlers(void) act.sa_sigaction = os_signal_handler; sigemptyset(&act.sa_mask); - act.sa_flags = SA_SIGINFO | SA_NODEFER; + act.sa_flags = SA_SIGINFO; if (sigaction(SIGILL, &act, NULL) || sigaction(SIGBUS, &act, NULL) || sigaction(SIGSEGV, &act, NULL))
The sandbox can handle signals. Due to a damaged global data pointer additional exceptions in the signal handler may occur leading to an endless loop. In this case leave the handling of the secondary exception to the operating system. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- arch/sandbox/cpu/os.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.30.2