1. Patchew
  2. linux
  3. View series

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

Uros Bizjak posted 1 patch 2 months, 1 week ago 
arch/x86/kernel/ptrace.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
[PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm
Posted by Uros Bizjak 2 months, 1 week ago 
Replace direct 'movl' instructions for DS, ES, FS, and GS read in
get_segment_reg() with the savesegment() helper. This improves
readability, consistency, and ensures proper handling of
segment registers on x86_64.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/kernel/ptrace.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 3dcadc13f09a..4cb00aa0645f 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -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;
 
 	switch (offset) {
 	case offsetof(struct user_regs_struct, fs):
 		if (task == current) {
-			/* Older gas can't assemble movq %?s,%r?? */
-			asm("movl %%fs,%0" : "=r" (seg));
-			return seg;
+			savesegment(fs, retval);
+			return retval;
 		}
 		return task->thread.fsindex;
 	case offsetof(struct user_regs_struct, gs):
 		if (task == current) {
-			asm("movl %%gs,%0" : "=r" (seg));
-			return seg;
+			savesegment(gs, retval);
+			return retval;
 		}
 		return task->thread.gsindex;
 	case offsetof(struct user_regs_struct, ds):
 		if (task == current) {
-			asm("movl %%ds,%0" : "=r" (seg));
-			return seg;
+			savesegment(ds, retval);
+			return retval;
 		}
 		return task->thread.ds;
 	case offsetof(struct user_regs_struct, es):
 		if (task == current) {
-			asm("movl %%es,%0" : "=r" (seg));
-			return seg;
+			savesegment(es, retval);
+			return retval;
 		}
 		return task->thread.es;
 
-- 
2.53.0
Re: [PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm
Posted by Oleg Nesterov 2 months, 1 week ago 
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.

Oleg.

>  
>  	switch (offset) {
>  	case offsetof(struct user_regs_struct, fs):
>  		if (task == current) {
> -			/* Older gas can't assemble movq %?s,%r?? */
> -			asm("movl %%fs,%0" : "=r" (seg));
> -			return seg;
> +			savesegment(fs, retval);
> +			return retval;
>  		}
>  		return task->thread.fsindex;
>  	case offsetof(struct user_regs_struct, gs):
>  		if (task == current) {
> -			asm("movl %%gs,%0" : "=r" (seg));
> -			return seg;
> +			savesegment(gs, retval);
> +			return retval;
>  		}
>  		return task->thread.gsindex;
>  	case offsetof(struct user_regs_struct, ds):
>  		if (task == current) {
> -			asm("movl %%ds,%0" : "=r" (seg));
> -			return seg;
> +			savesegment(ds, retval);
> +			return retval;
>  		}
>  		return task->thread.ds;
>  	case offsetof(struct user_regs_struct, es):
>  		if (task == current) {
> -			asm("movl %%es,%0" : "=r" (seg));
> -			return seg;
> +			savesegment(es, retval);
> +			return retval;
>  		}
>  		return task->thread.es;
>  
> -- 
> 2.53.0
>
Re: [PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm
Posted by Uros Bizjak 2 months, 1 week ago 
On Thu, Apr 2, 2026 at 2:48 PM Oleg Nesterov <oleg@redhat.com> 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.

Thanks,
Uros.
Re: [PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm
Posted by Uros Bizjak 2 months, 1 week ago 
On Thu, Apr 2, 2026 at 3:09 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Thu, Apr 2, 2026 at 2:48 PM Oleg Nesterov <oleg@redhat.com> 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):
Re: [PATCH] x86/ptrace: Use savesegment() in get_segment_reg() instead of inline asm
Posted by Oleg Nesterov 2 months, 1 week ago 
On 04/02, Uros Bizjak wrote:
>
> On Thu, Apr 2, 2026 at 3:09 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > 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.

OK, agreed. And this cleanup looks good to me as well.

Oleg.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.