lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 9 Oct 2020 09:13:20 -0600 From: Jens Axboe <axboe@...nel.dk> To: Oleg Nesterov <oleg@...hat.com> Cc: linux-kernel@...r.kernel.org, io-uring@...r.kernel.org, peterz@...radead.org, tglx@...utronix.de Subject: Re: [PATCH 3/4] kernel: add support for TIF_NOTIFY_SIGNAL On 10/9/20 8:43 AM, Oleg Nesterov wrote: > Once again, I am fine with this patch, just a minor comment... > > On 10/08, Jens Axboe wrote: >> >> --- a/arch/x86/kernel/signal.c >> +++ b/arch/x86/kernel/signal.c >> @@ -808,7 +808,10 @@ void arch_do_signal(struct pt_regs *regs) >> { >> struct ksignal ksig; >> >> - if (get_signal(&ksig)) { >> + if (test_thread_flag(TIF_NOTIFY_SIGNAL)) >> + tracehook_notify_signal(); >> + >> + if (task_sigpending(current) && get_signal(&ksig)) { > > I suggested to change arch_do_signal() because somehow I like it this way ;) > > And because we can easily pass the "ti_work" mask to arch_do_signal() and > avoid test_thread_flag/task_sigpending. > > Hmm. I just noticed that only x86 uses arch_do_signal(), so perhaps you can > add this change to this patch right now? Up to you. Sure, we can do that. Incremental on top then looks like the below. I don't feel that strongly about it, but it is nice to avoid re-testing the flags. diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index cd140bbf8520..ec6490e53dc3 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -804,14 +804,14 @@ static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs) * want to handle. Thus you cannot kill init even with a SIGKILL even by * mistake. */ -void arch_do_signal(struct pt_regs *regs) +void arch_do_signal(struct pt_regs *regs, unsigned long ti_work) { struct ksignal ksig; - if (test_thread_flag(TIF_NOTIFY_SIGNAL)) + if (ti_work & _TIF_NOTIFY_SIGNAL) tracehook_notify_signal(); - if (task_sigpending(current) && get_signal(&ksig)) { + if ((ti_work & _TIF_SIGPENDING) && get_signal(&ksig)) { /* Whee! Actually deliver the signal. */ handle_signal(&ksig, regs); return; diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index f4234aaac36c..0360b7e4e39a 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -265,10 +265,11 @@ static __always_inline void arch_exit_to_user_mode(void) { } /** * arch_do_signal - Architecture specific signal delivery function * @regs: Pointer to currents pt_regs + * @ti_work task thread info work flags * * Invoked from exit_to_user_mode_loop(). */ -void arch_do_signal(struct pt_regs *regs); +void arch_do_signal(struct pt_regs *regs, unsigned long ti_work); /** * arch_syscall_exit_tracehook - Wrapper around tracehook_report_syscall_exit() diff --git a/kernel/entry/common.c b/kernel/entry/common.c index 89a068252897..bd3cf6279e94 100644 --- a/kernel/entry/common.c +++ b/kernel/entry/common.c @@ -135,7 +135,7 @@ static __always_inline void exit_to_user_mode(void) } /* Workaround to allow gradual conversion of architecture code */ -void __weak arch_do_signal(struct pt_regs *regs) { } +void __weak arch_do_signal(struct pt_regs *regs, unsigned long ti_work) { } static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, unsigned long ti_work) @@ -158,7 +158,7 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, klp_update_patch_state(current); if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) - arch_do_signal(regs); + arch_do_signal(regs, ti_work); if (ti_work & _TIF_NOTIFY_RESUME) { tracehook_notify_resume(regs); -- Jens Axboe
Powered by blists - more mailing lists