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

[Ingo Molnar <mingo@kernel.org>]

* 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