[PATCH] powerpc: Fix exit_flags field placement in pt_regs for ptrace

"Mukesh Kumar Chaurasiya (IBM)" <[email protected]>
Newsgroups org.ozlabs.lists.linuxppc-dev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Commit d7a6797e0bc1 ("powerpc: add exit_flags field in pt_regs") added
the exit_flags field to struct pt_regs to pass internal exit control
flags (e.g. _TIF_RESTOREALL) from syscall_exit_prepare() to the
low-level assembly exit path.

However, the field was placed in a way that was visible to userspace
tools such as strace via PTRACE_GETREGS, or caused a struct layout or
size regression observable through ptrace. The field is purely
kernel-internal and must not be exposed beyond the user_pt_regs
boundary.

Move exit_flags into struct thread_info where it is only accessible to
the kernel, and keep it out of the ptrace-visible register window
entirely.

Fixes: d7a6797e0bc1 ("powerpc: add exit_flags field in pt_regs")
Reported-by: "Dmitry V. Levin" <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h      |  3 ---
 arch/powerpc/include/asm/thread_info.h |  1 +
 arch/powerpc/include/uapi/asm/ptrace.h | 14 ++++-------
 arch/powerpc/kernel/interrupt.c        | 32 +++++++++++++++-----------
 arch/powerpc/kernel/ptrace/ptrace.c    |  1 -
 arch/powerpc/kernel/signal.c           |  2 +-
 6 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index fdeb97421785..d53c4dd4d8b6 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -53,9 +53,6 @@ struct pt_regs
 				unsigned long esr;
 			};
 			unsigned long result;
-			unsigned long exit_flags;
-			/* Maintain 16 byte interrupt stack alignment */
-			unsigned long __pt_regs_pad[3];
 		};
 	};
 #if defined(CONFIG_PPC64) || defined(CONFIG_PPC_KUAP)
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index ee3b9adb5b67..0487e94d3416 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -57,6 +57,7 @@ struct thread_info {
 #ifdef CONFIG_SMP
 	unsigned int	cpu;
 #endif
+	unsigned long	exit_flags;		/* Exit Flags for entry/exit */
 	unsigned long	syscall_work;		/* SYSCALL_WORK_ flags */
 	unsigned long	local_flags;		/* private flags for thread */
 #ifdef CONFIG_LIVEPATCH_64
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index a393b7f2760a..01e630149d48 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -55,8 +55,6 @@ struct pt_regs
 	unsigned long dar;		/* Fault registers */
 	unsigned long dsisr;		/* on 4xx/Book-E used for ESR */
 	unsigned long result;		/* Result of a system call */
-	unsigned long exit_flags;	/* System call exit flags */
-	unsigned long __pt_regs_pad[3];	/* Maintain 16 byte interrupt stack alignment */
 };
 
 #endif /* __ASSEMBLER__ */
@@ -116,12 +114,10 @@ struct pt_regs
 #define PT_DAR	41
 #define PT_DSISR 42
 #define PT_RESULT 43
-#define PT_EXIT_FLAGS 44
-#define PT_PAD 47 /* 3 times */
-#define PT_DSCR 48
-#define PT_REGS_COUNT 48
+#define PT_DSCR 44
+#define PT_REGS_COUNT 44
 
-#define PT_FPR0	(PT_REGS_COUNT + 4)	/* each FP reg occupies 2 slots in this space */
+#define PT_FPR0	48	/* each FP reg occupies 2 slots in this space */
 
 #ifndef __powerpc64__
 
@@ -133,7 +129,7 @@ struct pt_regs
 #define PT_FPSCR (PT_FPR0 + 32)	/* each FP reg occupies 1 slot in 64-bit space */
 
 
-#define PT_VR0	(PT_FPSCR + 2)	/* <82> each Vector reg occupies 2 slots in 64-bit */
+#define PT_VR0 82	/* each Vector reg occupies 2 slots in 64-bit */
 #define PT_VSCR (PT_VR0 + 32*2 + 1)
 #define PT_VRSAVE (PT_VR0 + 33*2)
 
@@ -141,7 +137,7 @@ struct pt_regs
 /*
  * Only store first 32 VSRs here. The second 32 VSRs in VR0-31
  */
-#define PT_VSR0	(PT_VRSAVE + 2)	/* each VSR reg occupies 2 slots in 64-bit */
+#define PT_VSR0 150	/* each VSR reg occupies 2 slots in 64-bit */
 #define PT_VSR31 (PT_VSR0 + 2*31)
 #endif /* __powerpc64__ */
 
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index f04978080837..5b88bf72786c 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -89,15 +89,17 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
 					   long scv)
 {
 	unsigned long ti_flags;
+	unsigned long ret = 0;
 	bool is_not_scv = !IS_ENABLED(CONFIG_PPC_BOOK3S_64) || !scv;
 
 	kuap_assert_locked();
 
 	regs->result = r3;
-	regs->exit_flags = 0;
 
-	ti_flags = read_thread_flags();
+	/* Clear exit_flags so only flags set during this exit are visible */
+	current->thread_info.exit_flags = 0;
 
+	ti_flags = read_thread_flags();
 	if (unlikely(r3 >= (unsigned long)-MAX_ERRNO) && is_not_scv) {
 		if (likely(!(ti_flags & (_TIF_NOERROR | _TIF_RESTOREALL)))) {
 			r3 = -r3;
@@ -107,7 +109,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
 
 	if (unlikely(ti_flags & _TIF_PERSYSCALL_MASK)) {
 		if (ti_flags & _TIF_RESTOREALL)
-			regs->exit_flags = _TIF_RESTOREALL;
+			ret = _TIF_RESTOREALL;
 		else
 			regs->gpr[3] = r3;
 		clear_bits(_TIF_PERSYSCALL_MASK, &current_thread_info()->flags);
@@ -116,7 +118,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
 	}
 
 	if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
-		regs->exit_flags |= _TIF_RESTOREALL;
+		ret |= _TIF_RESTOREALL;
 	}
 
 	syscall_exit_to_user_mode(regs);
@@ -132,17 +134,19 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
 
 	/* Restore user access locks last */
 	kuap_user_restore(regs);
-
+	ret |= current->thread_info.exit_flags;
 #ifdef CONFIG_PPC64
-	regs->exit_result = regs->exit_flags;
+	regs->exit_result = ret;
 #endif
 
-	return regs->exit_flags;
+	return ret;
 }
 
 #ifdef CONFIG_PPC64
 notrace unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *regs)
 {
+	unsigned long ret;
+
 	/*
 	 * This is called when detecting a soft-pending interrupt as well as
 	 * an alternate-return interrupt. So we can't just have the alternate
@@ -167,9 +171,11 @@ notrace unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *reg
 	}
 
 	kuap_user_restore(regs);
-	regs->exit_result |= regs->exit_flags;
+	ret = current_thread_info()->exit_flags & _TIF_RESTOREALL;
+	current_thread_info()->exit_flags &= ~_TIF_RESTOREALL;
+	regs->exit_result |= ret;
 
-	return regs->exit_result;
+	return ret;
 }
 #endif
 
@@ -186,8 +192,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
 	 */
 	kuap_assert_locked();
 
+	/* Clear exit_flags so only flags set during this exit are visible */
+	current_thread_info()->exit_flags = 0;
+
 	local_irq_disable();
-	regs->exit_flags = 0;
 again:
 	check_return_regs_valid(regs);
 	user_enter_irqoff();
@@ -200,9 +208,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
 
 	/* Restore user access locks last */
 	kuap_user_restore(regs);
-
-	ret = regs->exit_flags;
-
+	ret = current_thread_info()->exit_flags & _TIF_RESTOREALL;
 #ifdef CONFIG_PPC64
 	regs->exit_result = ret;
 #endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 316d4f5ead8e..6cd180bc36ab 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -291,7 +291,6 @@ void __init pt_regs_check(void)
 	CHECK_REG(PT_DAR, dar);
 	CHECK_REG(PT_DSISR, dsisr);
 	CHECK_REG(PT_RESULT, result);
-	CHECK_REG(PT_EXIT_FLAGS, exit_flags);
 	#undef CHECK_REG
 
 	BUILD_BUG_ON(PT_REGS_COUNT != sizeof(struct user_pt_regs) / sizeof(unsigned long));
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index bb42a8b6c642..cc6498501610 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -356,6 +356,6 @@ void signal_fault(struct task_struct *tsk, struct pt_regs *regs,
 void arch_do_signal_or_restart(struct pt_regs *regs)
 {
 	BUG_ON(regs != current->thread.regs);
-	regs->exit_flags |= _TIF_RESTOREALL;
+	current_thread_info()->exit_flags |= _TIF_RESTOREALL;
 	do_signal(current);
 }
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.