| Message ID | 871s9j462u.fsf@xmission.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | signal/arm64: siginfo cleanups | expand |
Hi Eric, On Mon, Sep 24, 2018 at 11:07:05AM +0200, Eric W. Biederman wrote: > This is the continuation of my work to sort out signaling of exceptions > with siginfo. The old signal sending functions by taking a siginfo > argument resulted in their callers having to deal with the fiddly nature > of siginfo directly. In small numbers of callers this is not a problem > but in the number of callers in the kernel this resulted in cases > where fields were not initialized or improperly initialized before > being passed to userspace. > > To avoid having to worry about those issues I have added new signal > sending functions that each deal wit a different siginfo case. When > using these functions there is no room for the fiddly nature of siginfo > to cause mistakes. > > This is my set of changes to update arm64 to use those functions. > Along with some refactoring so those functions can be cleanly used. > > Folks please review and double check me. I think I have kept these > changes simple and obviously correct but I am human and mess up > sometimes. Nice clean-up, thanks. I started reviewing the patches, I should finish by tomorrow (I also applied them locally to give some testing). > After these patches have had a chance to be reviewed I plan to merge > them by my siginfo tree. If you would rather take them in the arm64 > tree let me know. All of the prerequisites should have been merged > through Linus's tree several releases ago. Either way works for me. There is a trivial conflict in force_signal_inject() with the arm64 for-next/core tree so I could as well put them on top of this branch and send them during the 4.20 merging window.
Catalin Marinas <catalin.marinas@arm.com> writes: > Hi Eric, > > On Mon, Sep 24, 2018 at 11:07:05AM +0200, Eric W. Biederman wrote: >> This is the continuation of my work to sort out signaling of exceptions >> with siginfo. The old signal sending functions by taking a siginfo >> argument resulted in their callers having to deal with the fiddly nature >> of siginfo directly. In small numbers of callers this is not a problem >> but in the number of callers in the kernel this resulted in cases >> where fields were not initialized or improperly initialized before >> being passed to userspace. >> >> To avoid having to worry about those issues I have added new signal >> sending functions that each deal wit a different siginfo case. When >> using these functions there is no room for the fiddly nature of siginfo >> to cause mistakes. >> >> This is my set of changes to update arm64 to use those functions. >> Along with some refactoring so those functions can be cleanly used. >> >> Folks please review and double check me. I think I have kept these >> changes simple and obviously correct but I am human and mess up >> sometimes. > > Nice clean-up, thanks. I started reviewing the patches, I should finish > by tomorrow (I also applied them locally to give some testing). > >> After these patches have had a chance to be reviewed I plan to merge >> them by my siginfo tree. If you would rather take them in the arm64 >> tree let me know. All of the prerequisites should have been merged >> through Linus's tree several releases ago. > > Either way works for me. There is a trivial conflict in > force_signal_inject() with the arm64 for-next/core tree so I could as > well put them on top of this branch and send them during the 4.20 > merging window. As long as there is a trivial conflict I would like to keep everything in one tree. There is a following patchset that manages to reduce the size of struct siginfo in the kernel that I have also posted for review. With everything in one tree I can make that change now, and just cross it off my list of things to worry about. Eric
On Thu, Sep 27, 2018 at 11:39:35AM +0200, Eric W. Biederman wrote: > Catalin Marinas <catalin.marinas@arm.com> writes: > > On Mon, Sep 24, 2018 at 11:07:05AM +0200, Eric W. Biederman wrote: > >> After these patches have had a chance to be reviewed I plan to merge > >> them by my siginfo tree. If you would rather take them in the arm64 > >> tree let me know. All of the prerequisites should have been merged > >> through Linus's tree several releases ago. > > > > Either way works for me. There is a trivial conflict in > > force_signal_inject() with the arm64 for-next/core tree so I could as > > well put them on top of this branch and send them during the 4.20 > > merging window. > > As long as there is a trivial conflict I would like to keep everything > in one tree. > > There is a following patchset that manages to reduce the size of struct > siginfo in the kernel that I have also posted for review. With > everything in one tree I can make that change now, and just cross it off > my list of things to worry about. Fine by me: Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> Tested-by: Catalin Marinas <catalin.marinas@arm.com> The conflict will appear in -next but the resolution is simple: diff --cc arch/arm64/kernel/traps.c index 21689c6a985f,856b32aa03d8..adb0a32c1568 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@@ -353,12 -366,6 +368,9 @@@ void force_signal_inject(int signal, in const char *desc; struct pt_regs *regs = current_pt_regs(); + if (WARN_ON(!user_mode(regs))) + return; + - clear_siginfo(&info); - switch (signal) { case SIGILL: desc = "undefined instruction";
This is the continuation of my work to sort out signaling of exceptions with siginfo. The old signal sending functions by taking a siginfo argument resulted in their callers having to deal with the fiddly nature of siginfo directly. In small numbers of callers this is not a problem but in the number of callers in the kernel this resulted in cases where fields were not initialized or improperly initialized before being passed to userspace. To avoid having to worry about those issues I have added new signal sending functions that each deal wit a different siginfo case. When using these functions there is no room for the fiddly nature of siginfo to cause mistakes. This is my set of changes to update arm64 to use those functions. Along with some refactoring so those functions can be cleanly used. Folks please review and double check me. I think I have kept these changes simple and obviously correct but I am human and mess up sometimes. After these patches have had a chance to be reviewed I plan to merge them by my siginfo tree. If you would rather take them in the arm64 tree let me know. All of the prerequisites should have been merged through Linus's tree several releases ago. Eric W. Biederman (15): signal/arm64: Push siginfo generation into arm64_notify_die signal/arm64: Remove unneeded tsk parameter from arm64_force_sig_info signal/arm64: Factor out arm64_show_signal from arm64_force_sig_info signal/arm64: Factor set_thread_esr out of __do_user_fault signal/arm64: Consolidate the two hwpoison cases in do_page_fault signal/arm64: For clarity separate the 3 signal sending cases in do_page_fault signal/arm64: Expand __do_user_fault and remove it signal/arm64: Only perform one esr_to_fault_info call in do_page_fault signal/arm64: Only call set_thread_esr once in do_page_fault signal/arm64: Add and use arm64_force_sig_fault where appropriate signal/arm64: Add and use arm64_force_sig_mceerr as appropriate signal/arm64: Remove arm64_force_sig_info signal/arm64: In ptrace_hbptriggered name the signal description string signal/arm64: Add and use arm64_force_sig_ptrace_errno_trap signal/arm64: Use send_sig_fault where appropriate arch/arm64/include/asm/system_misc.h | 3 +- arch/arm64/include/asm/traps.h | 5 +- arch/arm64/kernel/debug-monitors.c | 11 ++--- arch/arm64/kernel/fpsimd.c | 10 ++-- arch/arm64/kernel/ptrace.c | 16 +++--- arch/arm64/kernel/sys_compat.c | 13 ++--- arch/arm64/kernel/traps.c | 67 ++++++++++++------------- arch/arm64/mm/fault.c | 94 +++++++++++++----------------------- 8 files changed, 90 insertions(+), 129 deletions(-)