Re: [PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm

[Uros Bizjak]

On Thu, Apr 2, 2026 at 3:09 PM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
>
> On Thu, Apr 2, 2026 at 2:48 PM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >
> > On 04/02, Uros Bizjak wrote:
> > >
> > > @@ -251,32 +251,31 @@ static u16 get_segment_reg(struct task_struct *task, unsigned long offset)
> > >       /*
> > >        * Returning the value truncates it to 16 bits.
> > >        */
> > > -     unsigned int seg;
> > > +     unsigned int retval;
> >
> > LGTM, but perhaps it would be better to use "u16 retval" ? and remove the
> > comment.
>
> With the new definition of savesegment(), this is actually NOP from
> the compiler PoV.
>
> There is a corresponding x86_32 get_segment_reg() function that has
> the same definition of retval, I can prepare a follow-up patch that
> changes both.

Something like the attached patch that also slightly unifies x86_32 with x86_64.

Uros.
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 4cb00aa0645f..5fda7619fca6 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -182,19 +182,16 @@ static unsigned long *pt_regs_access(struct pt_regs *regs, unsigned long regno)
 
 static u16 get_segment_reg(struct task_struct *task, unsigned long offset)
 {
-	/*
-	 * Returning the value truncates it to 16 bits.
-	 */
-	unsigned int retval;
-	if (offset != offsetof(struct user_regs_struct, gs))
-		retval = *pt_regs_access(task_pt_regs(task), offset);
-	else {
-		if (task == current)
+	unsigned short retval;
+
+	if (offset == offsetof(struct user_regs_struct, gs)) {
+		if (task == current) {
 			savesegment(gs, retval);
-		else
-			retval = task->thread.gs;
+			return retval;
+		}
+		return task->thread.gs;
 	}
-	return retval;
+	return  *pt_regs_access(task_pt_regs(task), offset);
 }
 
 static int set_segment_reg(struct task_struct *task,
@@ -248,10 +245,7 @@ static unsigned long *pt_regs_access(struct pt_regs *regs, unsigned long offset)
 
 static u16 get_segment_reg(struct task_struct *task, unsigned long offset)
 {
-	/*
-	 * Returning the value truncates it to 16 bits.
-	 */
-	unsigned int retval;
+	unsigned short retval;
 
 	switch (offset) {
 	case offsetof(struct user_regs_struct, fs):