| Message ID | 20211020174406.17889-6-ebiederm@xmission.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | exit cleanups | expand |
On Wed, Oct 20, 2021 at 7:44 AM Eric W. Biederman <ebiederm@xmission.com> wrote: > > + force_sig(SIGKILL); I wonder if SIGFPE would be a more intuitive thing. Doesn't really matter, this is a "doesn't happen" event anyway, but that was just my reaction to reading the patch. Linus
On Wed, Oct 20, 2021 at 12:43:52PM -0500, Eric W. Biederman wrote: > Today the sh code allocates memory the first time a process uses > the fpu. If that memory allocation fails, kill the affected task > with force_sig(SIGKILL) rather than do_group_exit(SIGKILL). > > Calling do_group_exit from an exception handler can potentially lead > to dead locks as do_group_exit is not designed to be called from > interrupt context. Instead use force_sig(SIGKILL) to kill the > userspace process. Sending signals in general and force_sig in > particular has been tested from interrupt context so there should be > no problems. > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > Cc: Rich Felker <dalias@libc.org> > Cc: linux-sh@vger.kernel.org > Fixes: 0ea820cf9bf5 ("sh: Move over to dynamically allocated FPU context.") > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Looks sane; there should be no observable changes. Reviewed-by: Kees Cook <keescook@chromium.org>
On Wed, Oct 20, 2021 at 09:57:58AM -1000, Linus Torvalds wrote: > On Wed, Oct 20, 2021 at 7:44 AM Eric W. Biederman <ebiederm@xmission.com> wrote: > > > > + force_sig(SIGKILL); > > I wonder if SIGFPE would be a more intuitive thing. > > Doesn't really matter, this is a "doesn't happen" event anyway, but > that was just my reaction to reading the patch. I think SIGKILL makes more sense unless there's a way the process could handle the resulting SIGFPE and recover. I'd actually like to see the lazy allocation of FPU state just removed (the amount of space saved is tiny relative to the complexity cost and the negative aspects of unrecoverable late failure) but for now let's just go with this. Rich
diff --git a/arch/sh/kernel/cpu/fpu.c b/arch/sh/kernel/cpu/fpu.c index ae354a2931e7..fd6db0ab1928 100644 --- a/arch/sh/kernel/cpu/fpu.c +++ b/arch/sh/kernel/cpu/fpu.c @@ -62,18 +62,20 @@ void fpu_state_restore(struct pt_regs *regs) } if (!tsk_used_math(tsk)) { - local_irq_enable(); + int ret; /* * does a slab alloc which can sleep */ - if (init_fpu(tsk)) { + local_irq_enable(); + ret = init_fpu(tsk); + local_irq_disable(); + if (ret) { /* * ran out of memory! */ - do_group_exit(SIGKILL); + force_sig(SIGKILL); return; } - local_irq_disable(); } grab_fpu(regs);
Today the sh code allocates memory the first time a process uses the fpu. If that memory allocation fails, kill the affected task with force_sig(SIGKILL) rather than do_group_exit(SIGKILL). Calling do_group_exit from an exception handler can potentially lead to dead locks as do_group_exit is not designed to be called from interrupt context. Instead use force_sig(SIGKILL) to kill the userspace process. Sending signals in general and force_sig in particular has been tested from interrupt context so there should be no problems. Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Cc: linux-sh@vger.kernel.org Fixes: 0ea820cf9bf5 ("sh: Move over to dynamically allocated FPU context.") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/sh/kernel/cpu/fpu.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)