Date:   Mon, 12 Dec 2022 13:36:18 -0600
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     stsp <stsp2@...dex.ru>
Cc:     linux-kernel@...r.kernel.org, linux-x86_64@...r.kernel.org,
        luto@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, Al Viro <viro@...IV.linux.org.uk>
Subject: Re: strange behavior with sigreturn() to 32bit


Stas,

in your github report you mention that you believe this is a regression
https://github.com/dosemu2/dosemu2/pull/1830.

Can you tell us the last kernel this worked on?  Which kernel you tested
that this fails on?  It would be awesome if you could bisect this to the
commit that is broken but at least knowing kernel's you have tried that
work and don't work would be very useful.

As everything doesemu and vm86 does is pretty x86 centric I have added
the overall x86 maintainers as well as Al Viro in the hopes of this
getting routed to someone who has a bit more time then I do at the
moment.  I remember Al looking quite a bit at the architecture specific
signal handling bits.

Regressions where things used to work, but no longer work are much
more interesting than a simple bug report in a compatibility layer.

We have old bugs where 32bit compatibility code on 64bit kernels have
didn't always give the correct siginfo that I think I have fixed in
recent years.  I don't know if we have similar issues for sigcontext
which it seems that you are dealing with.

Dosemu is old enough that anything it has down historically that no
longer works certainly counts as a regression and should be fixed.

stsp <stsp2@...dex.ru> writes:

> I am playing with 32bit compatibility segments,
> and I am observing something very strange.
> To demonstrate the problem, I did the change
> to the kernel sigreturn test-case, and it is attached.
> The change just moves the magic value to EAX
> and calls an interrupt that produces a SIGSEGV.
> The SIGSEGV handler prints the needed regs.
> This patch intentionally adds 0x100000000 to
> the RIP register, because AFAIK the high part
> or 64bit regs are irrelevant in compatibility mode.
> Now with high part of RIP non-zero, we see this:
> $ ./sigreturn_64
> err=0 trapno=d ax=0 ip=100000003
>
> OK, obviously our asm code didn't execute -
> why is so? How does the high part of RIP
> affect compatibility mode?
> Then lets start the same example under gdb:
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000403008 in int31 ()
> (gdb)
> Continuing.
> err=18a trapno=d ax=a5f3 ip=403008
>
> Wow! Much better now: error code is correct,
> ax is correct, but ip is invalid.
>
> Now lets remove that high-part trashing from
> source and recompile, then run again:
> $ ./sigreturn_64
> err=0 trapno=d ax=a5f3 ip=6
>
> This time error code is incorrect! All other
> values are good.
> Lets see under gdb:
> err=18a trapno=d ax=a5f3 ip=403008
>
> Cool, under gdb the err code fixed itself but
> IP is wrong...
>
> Can anyone make any sense out of this? :)
> To me it seems completely chaotic and I
> suspect both sigreturn() and ptrace() are
> somehow "broken" here.
>
> diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c
> index 5d7961a5f7f6..6c8f2431d77d 100644
> --- a/tools/testing/selftests/x86/sigreturn.c
> +++ b/tools/testing/selftests/x86/sigreturn.c
> @@ -101,9 +101,14 @@ asm (".pushsection .text\n\t"
>       "mov %ss,%ecx\n\t"
>       "int3\n\t"
>       ".size int3, . - int3\n\t"
> +     ".type int31, @function\n\t"
> +     "int31:\n\t"
> +     "movl $0xa5f3, %eax\n\t"
> +     "int $0x31\n\t"
>       ".align 4096, 0xcc\n\t"
>       ".popsection");
>  extern char int3[4096];
> +extern char int31[];
>  
>  /*
>   * At startup, we prepapre:
> @@ -296,6 +301,7 @@ static volatile sig_atomic_t sig_corrupt_final_ss;
>  # define REG_IP REG_RIP
>  # define REG_SP REG_RSP
>  # define REG_CX REG_RCX
> +# define REG_AX REG_RAX
>  
>  struct selectors {
>  	unsigned short cs, gs, fs, ss;
> @@ -316,6 +322,7 @@ static unsigned short *csptr(ucontext_t *ctx)
>  # define REG_IP REG_EIP
>  # define REG_SP REG_ESP
>  # define REG_CX REG_ECX
> +# define REG_AX REG_EAX
>  
>  static greg_t *ssptr(ucontext_t *ctx)
>  {
> @@ -444,9 +451,12 @@ static void sigusr1(int sig, siginfo_t *info, void *ctx_void)
>  	*ssptr(ctx) = sig_ss;
>  
>  	ctx->uc_mcontext.gregs[REG_IP] =
> -		sig_cs == code16_sel ? 0 : (unsigned long)&int3;
> +		sig_cs == code16_sel ? ((unsigned long)&int31 -
> +			(unsigned long)&int3) | 0x100000000 :
> +		(unsigned long)&int3;
>  	ctx->uc_mcontext.gregs[REG_SP] = (unsigned long)0x8badf00d5aadc0deULL;
>  	ctx->uc_mcontext.gregs[REG_CX] = 0;
> +	ctx->uc_mcontext.gregs[REG_AX] = 0;
>  
>  #ifdef __i386__
>  	/*
> @@ -515,6 +525,20 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
>  	sig_trapped = sig;
>  }
>  
> +static void sigsegv(int sig, siginfo_t *info, void *ctx_void)
> +{
> +	ucontext_t *ctx = (ucontext_t*)ctx_void;
> +
> +	validate_signal_ss(sig, ctx);
> +
> +	sig_err = ctx->uc_mcontext.gregs[REG_ERR];
> +	sig_trapno = ctx->uc_mcontext.gregs[REG_TRAPNO];
> +	printf("err=%x trapno=%x ax=%x ip=%llx\n", sig_err, sig_trapno,
> +		(unsigned)ctx->uc_mcontext.gregs[REG_AX],
> +		(unsigned long long)ctx->uc_mcontext.gregs[REG_IP]);
> +	_exit(0);
> +}
> +
>  #ifdef __x86_64__
>  /* Tests recovery if !UC_STRICT_RESTORE_SS */
>  static void sigusr2(int sig, siginfo_t *info, void *ctx_void)
> @@ -777,6 +801,7 @@ int main()
>  
>  	sethandler(SIGUSR1, sigusr1, 0);
>  	sethandler(SIGTRAP, sigtrap, SA_ONSTACK);
> +	sethandler(SIGSEGV, sigsegv, SA_ONSTACK);
>  
>  	/* Easy cases: return to a 32-bit SS in each possible CS bitness. */
>  	total_nerrs += test_valid_sigreturn(64, false, -1);

Eric

Powered by blists - more mailing lists