Re: signal delivery, was Re: reliable reproducer

Michael Schmitz
2023-04-25T02:32:09Z

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,
>