Re: signal delivery, was Re: reliable reproducer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Finn,

On 25/04/23 14:32, Michael Schmitz wrote:
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;

Minimal gap that avoids the corruption for me (in your patch below) is 20 bytes. Still can't explain that (and I won't have time to throw more confusion into the discussion until maybe the weekend).

Cheers,

    Michael


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,




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux