Re: signal delivery, was Re: reliable reproducer

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Finn,

Am 25.04.2023 um 13:55 schrieb Finn Thain:
> On Tue, 25 Apr 2023, Finn Thain wrote:
>
>> On Tue, 25 Apr 2023, Michael Schmitz wrote:
>>
>>> As to a cause for the corruption: all the calculations in setup_frame
>>> and sys_sigreturn use fsize, but get_sigframe() masks off the result
>>> of usp - sizeof(sigframe) - fsize to place the entire frame at a
>>> quadword boundary.
>> ...
>>
>> I wonder if we are seeing some fallout from the issue described in
>> do_page_fault() i.e. usp is unreliable.
>>
>>                 /* Accessing the stack below usp is always a bug.  The
>>                    "+ 256" is there due to some instructions doing
>>                    pre-decrement on the stack and that doesn't show up
>>                    until later.  */
>>                 if (address + 256 < rdusp())
>>                         goto map_err;
>>
>> Maybe we should try modifying get_sigframe() to increase the gap between
>> the signal and exception frames from 0-1 long words up to 64-65 long
>> words.
>>
>
> It turns out that doing so (patch below) does make the problem go away.
> Was the exception frame getting clobbered?

Might happen, if the frame gap isn't actually equal to the exception 
frame extra size anymore? Aligning the start of the signal frame to the 
next lower quadword boundary increases the gap size.

When setting up the sigframe, the extra is copied to the correct 
location (right past struct sigframe, or into uc_filler). When moving 
that exception frame into place, the assumption is that the gap is the 
extra size, not more.

I'll try dropping the quadword alignment constraint - the return 
trampoline still ought to remain longword aligned.

Cheers,

	Michael


>
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index b9f6908a31bc..94104699f5a8 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -862,7 +862,7 @@ get_sigframe(struct ksignal *ksig, size_t frame_size)
>  {
>  	unsigned long usp = sigsp(rdusp(), ksig);
>
> -	return (void __user *)((usp - frame_size) & -8UL);
> +	return (void __user *)((usp - 256 - frame_size) & -8UL);
>  }
>
>  static int setup_frame(struct ksignal *ksig, sigset_t *set,
>