forked from mirrors/linux
		
	task_work: Call tracehook_notify_signal from get_signal on all architectures
Always handle TIF_NOTIFY_SIGNAL in get_signal.  With commit 35d0b389f3
("task_work: unconditionally run task_work from get_signal()") always
calling task_work_run all of the work of tracehook_notify_signal is
already happening except clearing TIF_NOTIFY_SIGNAL.
Factor clear_notify_signal out of tracehook_notify_signal and use it in
get_signal so that get_signal only needs one call of task_work_run.
To keep the semantics in sync update xfer_to_guest_mode_work (which
does not call get_signal) to call tracehook_notify_signal if either
_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL.
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lkml.kernel.org/r/20220309162454.123006-8-ebiederm@xmission.com
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
			
			
This commit is contained in:
		
							parent
							
								
									7f62d40d9c
								
							
						
					
					
						commit
						8ba62d3794
					
				
					 7 changed files with 18 additions and 29 deletions
				
			
		|  | @ -453,7 +453,7 @@ static void handle_signal(struct ksignal *ksig, sigset_t *oldset, | ||||||
|  * stack-frames in one go after that. |  * stack-frames in one go after that. | ||||||
|  */ |  */ | ||||||
| 
 | 
 | ||||||
| void arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal) | void arch_do_signal_or_restart(struct pt_regs *regs) | ||||||
| { | { | ||||||
| 	struct ksignal ksig; | 	struct ksignal ksig; | ||||||
| 	sigset_t *oldset = sigmask_to_save(); | 	sigset_t *oldset = sigmask_to_save(); | ||||||
|  | @ -466,7 +466,7 @@ void arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal) | ||||||
| 	current->thread.system_call = | 	current->thread.system_call = | ||||||
| 		test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0; | 		test_pt_regs_flag(regs, PIF_SYSCALL) ? regs->int_code : 0; | ||||||
| 
 | 
 | ||||||
| 	if (has_signal && get_signal(&ksig)) { | 	if (get_signal(&ksig)) { | ||||||
| 		/* Whee!  Actually deliver the signal.  */ | 		/* Whee!  Actually deliver the signal.  */ | ||||||
| 		if (current->thread.system_call) { | 		if (current->thread.system_call) { | ||||||
| 			regs->int_code = current->thread.system_call; | 			regs->int_code = current->thread.system_call; | ||||||
|  |  | ||||||
|  | @ -861,11 +861,11 @@ 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 |  * want to handle. Thus you cannot kill init even with a SIGKILL even by | ||||||
|  * mistake. |  * mistake. | ||||||
|  */ |  */ | ||||||
| void arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal) | void arch_do_signal_or_restart(struct pt_regs *regs) | ||||||
| { | { | ||||||
| 	struct ksignal ksig; | 	struct ksignal ksig; | ||||||
| 
 | 
 | ||||||
| 	if (has_signal && get_signal(&ksig)) { | 	if (get_signal(&ksig)) { | ||||||
| 		/* Whee! Actually deliver the signal.  */ | 		/* Whee! Actually deliver the signal.  */ | ||||||
| 		handle_signal(&ksig, regs); | 		handle_signal(&ksig, regs); | ||||||
| 		return; | 		return; | ||||||
|  |  | ||||||
|  | @ -257,7 +257,7 @@ static __always_inline void arch_exit_to_user_mode(void) { } | ||||||
|  * |  * | ||||||
|  * Invoked from exit_to_user_mode_loop(). |  * Invoked from exit_to_user_mode_loop(). | ||||||
|  */ |  */ | ||||||
| void arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal); | void arch_do_signal_or_restart(struct pt_regs *regs); | ||||||
| 
 | 
 | ||||||
| /**
 | /**
 | ||||||
|  * exit_to_user_mode - Fixup state when exiting to user mode |  * exit_to_user_mode - Fixup state when exiting to user mode | ||||||
|  |  | ||||||
|  | @ -106,6 +106,12 @@ static inline void tracehook_notify_resume(struct pt_regs *regs) | ||||||
| 	rseq_handle_notify_resume(NULL, regs); | 	rseq_handle_notify_resume(NULL, regs); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | static inline void clear_notify_signal(void) | ||||||
|  | { | ||||||
|  | 	clear_thread_flag(TIF_NOTIFY_SIGNAL); | ||||||
|  | 	smp_mb__after_atomic(); | ||||||
|  | } | ||||||
|  | 
 | ||||||
| /*
 | /*
 | ||||||
|  * called by exit_to_user_mode_loop() if ti_work & _TIF_NOTIFY_SIGNAL. This |  * called by exit_to_user_mode_loop() if ti_work & _TIF_NOTIFY_SIGNAL. This | ||||||
|  * is currently used by TWA_SIGNAL based task_work, which requires breaking |  * is currently used by TWA_SIGNAL based task_work, which requires breaking | ||||||
|  | @ -113,8 +119,7 @@ static inline void tracehook_notify_resume(struct pt_regs *regs) | ||||||
|  */ |  */ | ||||||
| static inline void tracehook_notify_signal(void) | static inline void tracehook_notify_signal(void) | ||||||
| { | { | ||||||
| 	clear_thread_flag(TIF_NOTIFY_SIGNAL); | 	clear_notify_signal(); | ||||||
| 	smp_mb__after_atomic(); |  | ||||||
| 	if (task_work_pending(current)) | 	if (task_work_pending(current)) | ||||||
| 		task_work_run(); | 		task_work_run(); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -139,15 +139,7 @@ void noinstr exit_to_user_mode(void) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /* Workaround to allow gradual conversion of architecture code */ | /* Workaround to allow gradual conversion of architecture code */ | ||||||
| void __weak arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal) { } | void __weak arch_do_signal_or_restart(struct pt_regs *regs) { } | ||||||
| 
 |  | ||||||
| static void handle_signal_work(struct pt_regs *regs, unsigned long ti_work) |  | ||||||
| { |  | ||||||
| 	if (ti_work & _TIF_NOTIFY_SIGNAL) |  | ||||||
| 		tracehook_notify_signal(); |  | ||||||
| 
 |  | ||||||
| 	arch_do_signal_or_restart(regs, ti_work & _TIF_SIGPENDING); |  | ||||||
| } |  | ||||||
| 
 | 
 | ||||||
| static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, | static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, | ||||||
| 					    unsigned long ti_work) | 					    unsigned long ti_work) | ||||||
|  | @ -170,7 +162,7 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, | ||||||
| 			klp_update_patch_state(current); | 			klp_update_patch_state(current); | ||||||
| 
 | 
 | ||||||
| 		if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) | 		if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) | ||||||
| 			handle_signal_work(regs, ti_work); | 			arch_do_signal_or_restart(regs); | ||||||
| 
 | 
 | ||||||
| 		if (ti_work & _TIF_NOTIFY_RESUME) | 		if (ti_work & _TIF_NOTIFY_RESUME) | ||||||
| 			tracehook_notify_resume(regs); | 			tracehook_notify_resume(regs); | ||||||
|  |  | ||||||
|  | @ -8,7 +8,7 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work) | ||||||
| 	do { | 	do { | ||||||
| 		int ret; | 		int ret; | ||||||
| 
 | 
 | ||||||
| 		if (ti_work & _TIF_NOTIFY_SIGNAL) | 		if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) | ||||||
| 			tracehook_notify_signal(); | 			tracehook_notify_signal(); | ||||||
| 
 | 
 | ||||||
| 		if (ti_work & _TIF_SIGPENDING) { | 		if (ti_work & _TIF_SIGPENDING) { | ||||||
|  |  | ||||||
|  | @ -2626,20 +2626,12 @@ bool get_signal(struct ksignal *ksig) | ||||||
| 	struct signal_struct *signal = current->signal; | 	struct signal_struct *signal = current->signal; | ||||||
| 	int signr; | 	int signr; | ||||||
| 
 | 
 | ||||||
|  | 	clear_notify_signal(); | ||||||
| 	if (unlikely(task_work_pending(current))) | 	if (unlikely(task_work_pending(current))) | ||||||
| 		task_work_run(); | 		task_work_run(); | ||||||
| 
 | 
 | ||||||
| 	/*
 |  | ||||||
| 	 * For non-generic architectures, check for TIF_NOTIFY_SIGNAL so |  | ||||||
| 	 * that the arch handlers don't all have to do it. If we get here |  | ||||||
| 	 * without TIF_SIGPENDING, just exit after running signal work. |  | ||||||
| 	 */ |  | ||||||
| 	if (!IS_ENABLED(CONFIG_GENERIC_ENTRY)) { |  | ||||||
| 		if (test_thread_flag(TIF_NOTIFY_SIGNAL)) |  | ||||||
| 			tracehook_notify_signal(); |  | ||||||
| 	if (!task_sigpending(current)) | 	if (!task_sigpending(current)) | ||||||
| 		return false; | 		return false; | ||||||
| 	} |  | ||||||
| 
 | 
 | ||||||
| 	if (unlikely(uprobe_deny_signal())) | 	if (unlikely(uprobe_deny_signal())) | ||||||
| 		return false; | 		return false; | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Eric W. Biederman
						Eric W. Biederman