lkml.org 
[lkml]   [2008]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH 1/3] signals: sigqueue_free: don't free sigqueue if it is queued
    Date
    The timer_delete/sigqueue_free behavior predates my involvement with the
    code. I think it is fine to leave pending signals queued. (POSIX makes it
    unspecified for deleted timers. A robust application cannot even assume
    that they will or won't be removed consistently on one running system.)

    What I think everyone agrees is wrong in the abstract is the status quo,
    where (effectively) signals stay queued but with zeroed siginfo_t values.
    (This violates POSIX.) If there is a signal, its siginfo_t must be intact.

    A robust application has to drain any pending timer signals by unblocking
    the signal or using sigwait. This is true if signals might remain
    properly queued as POSIX permits. It's also true of existing Linux
    kernels, where a bogus infoless signal remains pending.

    There is also the caveat I mentioned before about RLIMIT_SIGPENDING.
    An existing dumb application might do many iterations of timer_create,
    timer_settime, (timer fires), timer_delete, all with the timer signal
    blocked, and not expect that it can hit its rlimit and not be able to
    create more timers until it drains those signals. I don't mind breaking
    any such application. It's dumb.

    As I read it, POSIX leaves it the same unspecified what is the
    disposition of pending timer signals at the time of the implicit timer
    deletion at exec. But people sure find it surprising on exec.

    So maybe this (wholly untested)?
    The last hunk could go alone (just keep queued) before the rest (exec flush).


    Thanks,
    Roland


    diff --git a/kernel/signal.c b/kernel/signal.c
    index 72bb4f5..0000000 100644
    --- a/kernel/signal.c
    +++ b/kernel/signal.c
    @@ -205,18 +205,25 @@ static void __sigqueue_free(struct sigqu
    kmem_cache_free(sigqueue_cachep, q);
    }

    -void flush_sigqueue(struct sigpending *queue)
    +static void __flush_sigqueue(struct sigpending *queue, int timers)
    {
    struct sigqueue *q;

    sigemptyset(&queue->signal);
    while (!list_empty(&queue->list)) {
    q = list_entry(queue->list.next, struct sigqueue , list);
    + if (timers && q->info.si_code != SI_TIMER)
    + continue;
    list_del_init(&q->list);
    __sigqueue_free(q);
    }
    }

    +void flush_sigqueue(struct sigpending *queue)
    +{
    + __flush_sigqueue(queue, 0);
    +}
    +
    /*
    * Flush all pending signals for a task.
    */
    @@ -243,6 +250,7 @@ void ignore_signals(struct task_struct *

    /*
    * Flush all handlers for a task.
    + * Also flush all SI_TIMER signals from the queues.
    */

    void
    @@ -257,6 +265,9 @@ flush_signal_handlers(struct task_struct
    sigemptyset(&ka->sa.sa_mask);
    ka++;
    }
    +
    + __flush_sigqueue(&t->pending, 1);
    + __flush_sigqueue(&t->signal->shared_pending, 1);
    }

    int unhandled_signal(struct task_struct *tsk, int sig)
    @@ -1240,17 +1251,18 @@ void sigqueue_free(struct sigqueue *q)

    BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
    /*
    - * If the signal is still pending remove it from the
    - * pending queue. We must hold ->siglock while testing
    + * If the signal is still pending, leave it to be dequeued.
    + * We must hold ->siglock while testing
    * q->list to serialize with collect_signal().
    */
    spin_lock_irqsave(lock, flags);
    + q->flags &= ~SIGQUEUE_PREALLOC;
    if (!list_empty(&q->list))
    - list_del_init(&q->list);
    + q = NULL;
    spin_unlock_irqrestore(lock, flags);

    - q->flags &= ~SIGQUEUE_PREALLOC;
    - __sigqueue_free(q);
    + if (q)
    + __sigqueue_free(q);
    }

    int send_sigqueue(struct sigqueue *q, struct task_struct *t, int group)

    \
     
     \ /
      Last update: 2008-05-22 01:05    [W:0.041 / U:0.980 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site