| Message ID | 20260915-work-fd-reserve-unify-folded-v1-18-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: - [Medium] xtensa: inconsistent mid-syscall ptrace exit tracing due to stale flags -- commit f64c69fbd6ff1a2ee8dd26645f9d56d2d371647e Author: Christian Brauner <brauner@kernel.org> xtensa: commit fds on syscall exit Add TIF_FD_SLOTS and test it once the system call has stored its return value. A task that reserved descriptors uses the traced path. [ ... ] > diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S > index 272fff587907f..4b95f48736e6b 100644 > --- a/arch/xtensa/kernel/entry.S > +++ b/arch/xtensa/kernel/entry.S > @@ -1976,6 +1976,11 @@ ENTRY(system_call) > 1: /* regs->areg[2] = return_value */ > > s32i abi_rv, abi_saved0, PT_AREG2 > + > + /* descriptors reserved by the syscall are committed in do_syscall_trace_leave */ > + GET_THREAD_INFO(a4, a1) > + l32i a4, a4, TI_FLAGS > + bbsi.l a4, TIF_FD_SLOTS, 1f > bnez abi_saved1, 1f [Severity: Medium] Does this introduce a regression leading to inconsistent ptrace observability? The exit path in system_call() dynamically checks TIF_FD_SLOTS from fresh thread flags (loaded into a4), but falls back to abi_saved1 for trace work. Earlier in system_call(), abi_saved1 is cached before executing the syscall: GET_THREAD_INFO(a4, a1) l32i abi_saved1, a4, TI_FLAGS movi a4, _TIF_WORK_MASK and abi_saved1, abi_saved1, a4 If a tracer attaches mid-syscall (e.g. using PTRACE_ATTACH on an Xtensa process currently blocked inside a system call), wouldn't the exit trace events be missed for most syscalls because abi_saved1 lacks the new flags, yet mysteriously be caught if the syscall happens to allocate a file descriptor (because TIF_FD_SLOTS is checked against the fresh TI_FLAGS)? Could we reload the fresh _TIF_WORK_MASK from thread_info here instead of relying on the stale abi_saved1 cache?
diff --git a/arch/xtensa/include/asm/thread_info.h b/arch/xtensa/include/asm/thread_info.h index 5b74dfc35ef9..0c8eb5dc8398 100644 --- a/arch/xtensa/include/asm/thread_info.h +++ b/arch/xtensa/include/asm/thread_info.h @@ -124,6 +124,7 @@ static __always_inline struct thread_info *current_thread_info(void) #define TIF_SYSCALL_AUDIT 9 /* syscall auditing active */ #define TIF_SECCOMP 10 /* secure computing */ #define TIF_MEMDIE 11 /* is terminating due to OOM killer */ +#define TIF_FD_SLOTS 12 /* syscall prepared descriptors */ #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) #define _TIF_SIGPENDING (1<<TIF_SIGPENDING) @@ -134,6 +135,7 @@ static __always_inline struct thread_info *current_thread_info(void) #define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME) #define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) #define _TIF_SECCOMP (1<<TIF_SECCOMP) +#define _TIF_FD_SLOTS (1<<TIF_FD_SLOTS) #define _TIF_WORK_MASK (_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP | \ _TIF_SYSCALL_TRACEPOINT | \ diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S index 272fff587907..4b95f48736e6 100644 --- a/arch/xtensa/kernel/entry.S +++ b/arch/xtensa/kernel/entry.S @@ -1976,6 +1976,11 @@ ENTRY(system_call) 1: /* regs->areg[2] = return_value */ s32i abi_rv, abi_saved0, PT_AREG2 + + /* descriptors reserved by the syscall are committed in do_syscall_trace_leave */ + GET_THREAD_INFO(a4, a1) + l32i a4, a4, TI_FLAGS + bbsi.l a4, TIF_FD_SLOTS, 1f bnez abi_saved1, 1f .Lsyscall_exit: #if defined(__XTENSA_WINDOWED_ABI__) diff --git a/arch/xtensa/kernel/ptrace.c b/arch/xtensa/kernel/ptrace.c index 364e4fdabb00..d44d62a56b85 100644 --- a/arch/xtensa/kernel/ptrace.c +++ b/arch/xtensa/kernel/ptrace.c @@ -13,6 +13,7 @@ */ #include <linux/audit.h> +#include <linux/file.h> #include <linux/errno.h> #include <linux/hw_breakpoint.h> #include <linux/kernel.h> @@ -571,6 +572,9 @@ void do_syscall_trace_leave(struct pt_regs *regs) { int step; + if (test_thread_flag(TIF_FD_SLOTS)) + fd_slots_commit(regs); + audit_syscall_exit(regs); if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
Add TIF_FD_SLOTS and test it once the system call has stored its return value. A task that reserved descriptors uses the traced path. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> --- arch/xtensa/include/asm/thread_info.h | 2 ++ arch/xtensa/kernel/entry.S | 5 +++++ arch/xtensa/kernel/ptrace.c | 4 ++++ 3 files changed, 11 insertions(+)