Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1334952 > unrolled thread

[PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs Andy Lutomirski <luto@kernel.org> - 2016-02-16 03:40 +0100
    Re: [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs  and gs Ingo Molnar <mingo@kernel.org> - 2016-02-16 08:50 +0100
      Re: [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs  and gs Andy Lutomirski <luto@amacapital.net> - 2016-02-16 16:00 +0100

#1334952 — [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

These fields have a strange history.  This tries to document it.

This borrows from 9a036b93a344 ("x86/signal/64: Remove 'fs' and 'gs'
from sigcontext"), which was reverted by ed596cde9425 ("Revert x86
sigcontext cleanups").

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/uapi/asm/sigcontext.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h
index d485232f1e9f..47dae8150520 100644
--- a/arch/x86/include/uapi/asm/sigcontext.h
+++ b/arch/x86/include/uapi/asm/sigcontext.h
@@ -341,6 +341,25 @@ struct sigcontext {
 	__u64				rip;
 	__u64				eflags;		/* RFLAGS */
 	__u16				cs;
+
+	/*
+	 * Prior to 2.5.64 ("[PATCH] x86-64 updates for 2.5.64-bk3"),
+	 * Linux saved and restored fs and gs in these slots.  This
+	 * was counterproductive, as fsbase and gsbase were never
+	 * saved, so arch_prctl was presumably unreliable.
+	 *
+	 * If these slots are ever needed for any other purpose, there
+	 * is some risk that very old 64-bit binaries could get
+	 * confused.  I doubt that many such binaries still work,
+	 * though, since the same patch in 2.5.64 also removed the
+	 * 64-bit set_thread_area syscall, so it appears that there is
+	 * no TLS API beyond modify_ldt that works in both pre- and
+	 * post-2.5.64 kernels.
+	 *
+	 * There is at least one additional concern if these slots are
+	 * recycled for another purpose: some DOSEMU versions stash fs
+	 * and gs in these slots manually.
+	 */
 	__u16				gs;
 	__u16				fs;
 	__u16				__pad0;
-- 
2.5.0

[toc] | [next] | [standalone]

#1335102 — Re: [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

* Andy Lutomirski <luto@kernel.org> wrote:

> These fields have a strange history.  This tries to document it.
> 
> This borrows from 9a036b93a344 ("x86/signal/64: Remove 'fs' and 'gs'
> from sigcontext"), which was reverted by ed596cde9425 ("Revert x86
> sigcontext cleanups").
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/uapi/asm/sigcontext.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h
> index d485232f1e9f..47dae8150520 100644
> --- a/arch/x86/include/uapi/asm/sigcontext.h
> +++ b/arch/x86/include/uapi/asm/sigcontext.h
> @@ -341,6 +341,25 @@ struct sigcontext {
>  	__u64				rip;
>  	__u64				eflags;		/* RFLAGS */
>  	__u16				cs;
> +
> +	/*
> +	 * Prior to 2.5.64 ("[PATCH] x86-64 updates for 2.5.64-bk3"),
> +	 * Linux saved and restored fs and gs in these slots.  This
> +	 * was counterproductive, as fsbase and gsbase were never
> +	 * saved, so arch_prctl was presumably unreliable.
> +	 *
> +	 * If these slots are ever needed for any other purpose, there
> +	 * is some risk that very old 64-bit binaries could get
> +	 * confused.  I doubt that many such binaries still work,
> +	 * though, since the same patch in 2.5.64 also removed the
> +	 * 64-bit set_thread_area syscall, so it appears that there is
> +	 * no TLS API beyond modify_ldt that works in both pre- and
> +	 * post-2.5.64 kernels.
> +	 *
> +	 * There is at least one additional concern if these slots are
> +	 * recycled for another purpose: some DOSEMU versions stash fs
> +	 * and gs in these slots manually.
> +	 */
>  	__u16				gs;
>  	__u16				fs;

So I think this comment should be a lot more assertive: it should state that due 
to these old legacies that user-space learned to rely on the kernel must not touch 
these fields. I.e. it is an ABI - no ifs and whens.

We should also rename them to __dosemu_gs_reserved/__dosemu_fs_reserved or so. 
These are ABI legacies for DOSEMU, no need to pretend otherwise. There's very 
little to be sorry about: ABI promises have consequences, we should codify that 
here and move on. Also please document it precisely which syscall(s) expose this 
ABI.

If we need more space for new, cleaner functionality we'll use other fields. (like 
your later patches do.)

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1335483 — Re: [PATCH v4 1/4] x86/signal/64: Add a comment about sigcontext->fs and gs

On Feb 16, 2016 12:42 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
>
>
> * Andy Lutomirski <luto@kernel.org> wrote:
>
> > These fields have a strange history.  This tries to document it.
> >
> > This borrows from 9a036b93a344 ("x86/signal/64: Remove 'fs' and 'gs'
> > from sigcontext"), which was reverted by ed596cde9425 ("Revert x86
> > sigcontext cleanups").
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  arch/x86/include/uapi/asm/sigcontext.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h
> > index d485232f1e9f..47dae8150520 100644
> > --- a/arch/x86/include/uapi/asm/sigcontext.h
> > +++ b/arch/x86/include/uapi/asm/sigcontext.h
> > @@ -341,6 +341,25 @@ struct sigcontext {
> >       __u64                           rip;
> >       __u64                           eflags;         /* RFLAGS */
> >       __u16                           cs;
> > +
> > +     /*
> > +      * Prior to 2.5.64 ("[PATCH] x86-64 updates for 2.5.64-bk3"),
> > +      * Linux saved and restored fs and gs in these slots.  This
> > +      * was counterproductive, as fsbase and gsbase were never
> > +      * saved, so arch_prctl was presumably unreliable.
> > +      *
> > +      * If these slots are ever needed for any other purpose, there
> > +      * is some risk that very old 64-bit binaries could get
> > +      * confused.  I doubt that many such binaries still work,
> > +      * though, since the same patch in 2.5.64 also removed the
> > +      * 64-bit set_thread_area syscall, so it appears that there is
> > +      * no TLS API beyond modify_ldt that works in both pre- and
> > +      * post-2.5.64 kernels.
> > +      *
> > +      * There is at least one additional concern if these slots are
> > +      * recycled for another purpose: some DOSEMU versions stash fs
> > +      * and gs in these slots manually.
> > +      */
> >       __u16                           gs;
> >       __u16                           fs;
>
> So I think this comment should be a lot more assertive: it should state that due
> to these old legacies that user-space learned to rely on the kernel must not touch
> these fields. I.e. it is an ABI - no ifs and whens.

We could still touch them to a limited extent.  For example, we could
save FS and GS there (but we probably can't restore them).

I'll improve the comment.

>
> We should also rename them to __dosemu_gs_reserved/__dosemu_fs_reserved or so.

I suspect that DOSEMU won't build if we do that.  In any event, I
think it should be a separate patch so that it can be trivially
reverted if needed.

--Andy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web