On 03/20, Hiroshi Shimamoto wrote:
>
> Commit-ID: 14fc9fbc700dc95b4f46ebd588169324fe6deff8
> Gitweb: http://git.kernel.org/tip/14fc9fbc700dc95b4f46ebd588169324fe6deff8
> Author: Hiroshi Shimamoto <h-shimamoto@xxxxxxxxxxxxx>
> AuthorDate: Thu, 19 Mar 2009 10:56:29 -0700
> Committer: Ingo Molnar <mingo@xxxxxxx>
> CommitDate: Fri, 20 Mar 2009 19:01:31 +0100
>
> x86: signal: check signal stack overflow properly
>
> Impact: cleanup
>
> Check alternate signal stack overflow with proper stack pointer.
> The stack pointer of the next signal frame is different if that
> task has i387 state.
I think the patch is correct but I have a minor question,
> No need to check SA_ONSTACK if we're already using alternate signal stack.
Yes, but this also mean that we don't need sas_ss_flags() under
"if (!onsigstack)",
> @@ -211,31 +211,27 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
> {
> /* Default to using normal stack */
> unsigned long sp = regs->sp;
> + int onsigstack = on_sig_stack(sp);
>
> #ifdef CONFIG_X86_64
> /* redzone */
> sp -= 128;
> #endif /* CONFIG_X86_64 */
>
> - /*
> - * If we are on the alternate signal stack and would overflow it, don't.
> - * Return an always-bogus address instead so we will die with SIGSEGV.
> - */
> - if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
> - return (void __user *) -1L;
> -
> - /* This is the X/Open sanctioned signal stack switching. */
> - if (ka->sa.sa_flags & SA_ONSTACK) {
> - if (sas_ss_flags(sp) == 0)
> - sp = current->sas_ss_sp + current->sas_ss_size;
> - } else {
> + if (!onsigstack) {
> + /* This is the X/Open sanctioned signal stack switching. */
> + if (ka->sa.sa_flags & SA_ONSTACK) {
> + if (sas_ss_flags(sp) == 0)
> + sp = current->sas_ss_sp + current->sas_ss_size;
We can use "->sas_ss_size != 0" instead and avoid the unnecessary
sas_ss_flags()->on_sig_stack() check.
Please note that afaics sas_ss_flags()->on_sig_stack() is actually
wrong because we already adjusted "sp" above for redzone.
Suppose that on_sig_stack(regs->sp) = F, but "sp - 128" falls into
the altstack. In that case SA_ONSTACK won't switch the stack.
Of course, this is only theoretical, but still.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|