Re: [PATCH RFC v1] m68k: signal.c: use signal frame gap to avoid stack corruption

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

 



Hi Finn,

Am 30.04.2023 um 12:24 schrieb Finn Thain:

On Sun, 30 Apr 2023, Michael Schmitz wrote:

Am 29.04.2023 um 21:49 schrieb Finn Thain:


That seems over-complicated to me... I must be missing something.


If signals don't nest, a page fault on the signal stack should never
produce a new signal frame there, so this bug should never show up there.

I'm sure it doesn't. I'd just seen problems in an earlier version where none of these checks were in place. Using a signal stack location based on the fault address without making sure the fault actually happened on the user stack (i.e. below USP but not too far below) means we may place the signal stack in the text segment or static data or the heap area ...

I think that was the point I'd missed, because it means the signal stack
may be exempted here. But is the extra complexity worth it?

Any area that a bus fault happened on, except that right below USP should be exempted.

But your point about complexity is well taken. I must be missing another case where signal stack placement based on fault address is incorrect (kernel fault in __clear_user called from padzero when loading an ELF binary - odd ...)

Back to the drawing board.

What I had in mind was something like this (untested):

unsigned long usp;

if (CPU_IS_020_OR_030 && tregs->format == 0xb)
    /* USP is unreliable so use the worst-case value. */
    usp = sigsp(rdusp() - 256, ksig);
else
    usp = sigsp(rdusp(), ksig);

return (void __user *)((usp - frame_size) & -8UL);

sigsp() may return an address from the alternate signal stack. In that
case, no adjustment to the signal stack address is advised.
...

It'd be ill advised as it would reduce data locality when it expanded the
signal stack across a cache line or page boundary, and there may be a

You can't expand the alternate signal stack. It's obtained by malloc(), and we can't just hope that extending it down another page 'just works' in the same way as when we use the user stack (even there, it will only work up to the processes' stack limit).

performance penalty for that. However, the signal stack seems to be prone
to poor data locality anyway, compared to the normal user stack. So I
wonder if this penalty could be measured. It seems like a premature
optimization to me. Unfortunately, I don't think we get to measure cache
misses on m68k.

Yep, the whole scheme does look overly complex with a little more thought (and testing).

A fixed offset, or skipping signal delivery entirely is much the easiest solution for now.

Cheers,

	Michael



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

  Powered by Linux