| Message ID | 20260915-work-fd-reserve-unify-folded-v1-14-4d5217d6b246@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | file: handle files on syscall exit | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] parisc: syscall fast path exposes uninitialized pt_regs to ptrace -- commit 38a139df8c7c61e11fc9314b5f43e371c71a8356 Author: Christian Brauner <brauner@kernel.org> parisc: commit fds on syscall exit This commit adds TIF_FD_SLOTS and tests it in syscall_exit once the return value has been saved. A task that reserved descriptors calls do_syscall_trace_exit() to commit descriptors and then continues with the usual checks. > diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S > index e04c5d806c105..c5807cf6fbd8d 100644 > --- a/arch/parisc/kernel/entry.S > +++ b/arch/parisc/kernel/entry.S [ ... ] > @@ -1765,6 +1767,16 @@ syscall_do_signal: > > b,n syscall_check_sig > > +syscall_do_fd_slots: > + mfctl %cr30,%r1 > +#ifdef CONFIG_64BIT > + ldo -16(%r30),%r29 /* Reference param save area */ > +#endif > + ldo TASK_REGS(%r1),%r26 /* struct pt_regs *regs */ > + BL do_syscall_trace_exit,%r2 [Severity: High] Does this expose uninitialized kernel stack memory to a tracer? When a process executes a system call that reserves a file descriptor slot, it enters via the fast path which only saves a minimal set of registers to TASK_REGS. If TIF_FD_SLOTS is set, the fast path directly invokes do_syscall_trace_exit() here before all registers (like %r3-%r18) are saved. > + nop > + b,n syscall_check_resched > + > syscall_restore: > mfctl %cr30,%r1 > [ ... ] > diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c > index 1d9e210702e1f..f142766792afa 100644 > --- a/arch/parisc/kernel/ptrace.c > +++ b/arch/parisc/kernel/ptrace.c [ ... ] > @@ -381,6 +382,9 @@ void do_syscall_trace_exit(struct pt_regs *regs) > int stepping = test_thread_flag(TIF_SINGLESTEP) || > test_thread_flag(TIF_BLOCKSTEP); > > + if (test_thread_flag(TIF_FD_SLOTS)) > + fd_slots_commit(regs); > + > audit_syscall_exit(regs); > > #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS [ ... ] > if (stepping || test_thread_flag(TIF_SYSCALL_TRACE)) > ptrace_report_syscall_exit(regs, stepping); > } [Severity: High] If a tracer attaches concurrently while the process is blocked in a file descriptor allocation syscall, setting TIF_SYSCALL_TRACE, won't this call pass the incompletely initialized pt_regs from the fast path to the tracing subsystem? This could allow an unprivileged user attaching a tracer to their own process to read uninitialized kernel memory via PTRACE_GETREGS. Also, tracer modifications via PTRACE_SETREGS to these uninitialized registers would be ignored because the fast path does not properly restore them before returning to userspace.
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h index b283738bb6da..14a40c53689f 100644 --- a/arch/parisc/include/asm/thread_info.h +++ b/arch/parisc/include/asm/thread_info.h @@ -52,6 +52,7 @@ struct thread_info { #define TIF_SECCOMP 11 /* secure computing */ #define TIF_SYSCALL_TRACEPOINT 12 /* syscall tracepoint instrumentation */ #define TIF_NONBLOCK_WARNING 13 /* warned about wrong O_NONBLOCK usage */ +#define TIF_FD_SLOTS 14 /* syscall prepared descriptors */ #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) @@ -65,6 +66,7 @@ struct thread_info { #define _TIF_BLOCKSTEP (1 << TIF_BLOCKSTEP) #define _TIF_SECCOMP (1 << TIF_SECCOMP) #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) +#define _TIF_FD_SLOTS (1 << TIF_FD_SLOTS) #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | \ _TIF_NEED_RESCHED | _TIF_NOTIFY_SIGNAL) diff --git a/arch/parisc/kernel/entry.S b/arch/parisc/kernel/entry.S index e04c5d806c10..c5807cf6fbd8 100644 --- a/arch/parisc/kernel/entry.S +++ b/arch/parisc/kernel/entry.S @@ -1732,6 +1732,8 @@ syscall_check_resched: /* check for reschedule */ mfctl %cr30,%r19 LDREG TASK_TI_FLAGS(%r19),%r19 /* long */ + /* descriptors reserved by the syscall are committed in do_syscall_trace_exit */ + bb,<,n %r19, 31-TIF_FD_SLOTS, syscall_do_fd_slots /* forward */ bb,<,n %r19, 31-TIF_NEED_RESCHED, syscall_do_resched /* forward */ .import do_signal,code @@ -1765,6 +1767,16 @@ syscall_do_signal: b,n syscall_check_sig +syscall_do_fd_slots: + mfctl %cr30,%r1 +#ifdef CONFIG_64BIT + ldo -16(%r30),%r29 /* Reference param save area */ +#endif + ldo TASK_REGS(%r1),%r26 /* struct pt_regs *regs */ + BL do_syscall_trace_exit,%r2 + nop + b,n syscall_check_resched + syscall_restore: mfctl %cr30,%r1 diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c index 1d9e210702e1..f142766792af 100644 --- a/arch/parisc/kernel/ptrace.c +++ b/arch/parisc/kernel/ptrace.c @@ -9,6 +9,7 @@ */ #include <linux/kernel.h> +#include <linux/file.h> #include <linux/sched.h> #include <linux/mm.h> #include <linux/smp.h> @@ -381,6 +382,9 @@ void do_syscall_trace_exit(struct pt_regs *regs) int stepping = test_thread_flag(TIF_SINGLESTEP) || test_thread_flag(TIF_BLOCKSTEP); + if (test_thread_flag(TIF_FD_SLOTS)) + fd_slots_commit(regs); + audit_syscall_exit(regs); #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
Add TIF_FD_SLOTS and test it in syscall_exit once the return value has been saved to pt_regs. A task that reserved descriptors calls do_syscall_trace_exit() and then continues with the usual reschedule and signal checks. The traced path already ends up there. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> --- arch/parisc/include/asm/thread_info.h | 2 ++ arch/parisc/kernel/entry.S | 12 ++++++++++++ arch/parisc/kernel/ptrace.c | 4 ++++ 3 files changed, 18 insertions(+)