lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 24 Aug 2007 10:26:28 -0400 From: taoyue <yue.tao@...driver.com> To: Oleg Nesterov <oleg@...sign.ru> CC: Andrew Morton <akpm@...ux-foundation.org>, Alexey Dobriyan <adobriyan@...ru>, Ingo Molnar <mingo@...e.hu>, Jeremy Katz <jeremy.katz@...driver.com>, Thomas Gleixner <tglx@...utronix.de>, Roland McGrath <roland@...hat.com>, linux-kernel@...r.kernel.org, stable@...nel.org Subject: Re: [PATCH] sigqueue_free: fix the race with collect_signal() Oleg Nesterov wrote: > Spotted by taoyue <yue.tao@...driver.com> and Jeremy Katz <jeremy.katz@...driver.com>. > > collect_signal: sigqueue_free: > > list_del_init(&first->list); > if (!list_empty(&q->list)) { > // not taken > } > q->flags &= ~SIGQUEUE_PREALLOC; > > __sigqueue_free(first); __sigqueue_free(q); > > Now, __sigqueue_free() is called twice on the same "struct sigqueue" with the > obviously bad implications. > > In particular, this double free breaks the array_cache->avail logic, so the > same sigqueue could be "allocated" twice, and the bug can manifest itself via > the "impossible" BUG_ON(!SIGQUEUE_PREALLOC) in sigqueue_free/send_sigqueue. > > Hopefully this can explain these mysterious bug-reports, see > > http://marc.info/?t=118766926500003 > http://marc.info/?t=118466273000005 > > Alexey Dobriyan reports this patch makes the difference for the testcase, but > nobody has an access to the application which opened the problems originally. > > Also, this patch removes tasklist lock/unlock, ->siglock is enough. > > Signed-off-by: Oleg Nesterov <oleg@...sign.ru> > > --- t/kernel/signal.c~SQFREE 2007-08-22 20:06:31.000000000 +0400 > +++ t/kernel/signal.c 2007-08-23 16:02:57.000000000 +0400 > @@ -1297,20 +1297,19 @@ struct sigqueue *sigqueue_alloc(void) > void sigqueue_free(struct sigqueue *q) > { > unsigned long flags; > + spinlock_t *lock = ¤t->sighand->siglock; > + > BUG_ON(!(q->flags & SIGQUEUE_PREALLOC)); > /* > * If the signal is still pending remove it from the > - * pending queue. > + * pending queue. We must hold ->siglock while testing > + * q->list to serialize with collect_signal(). > */ > - if (unlikely(!list_empty(&q->list))) { > - spinlock_t *lock = ¤t->sighand->siglock; > - read_lock(&tasklist_lock); > - spin_lock_irqsave(lock, flags); > - if (!list_empty(&q->list)) > - list_del_init(&q->list); > - spin_unlock_irqrestore(lock, flags); > - read_unlock(&tasklist_lock); > - } > + spin_lock_irqsave(lock, flags); > + if (!list_empty(&q->list)) > + list_del_init(&q->list); > + spin_unlock_irqrestore(lock, flags); > + > q->flags &= ~SIGQUEUE_PREALLOC; > __sigqueue_free(q); > } > > > Applying previous patch,it seems likely that the __sigqueue_free() is also called twice. collect_signal: sigqueue_free: list_del_init(&first->list); spin_lock_irqsave(lock, flags); if (!list_empty(&q->list)) list_del_init(&q->list); spin_unlock_irqrestore(lock, flags); q->flags &= ~SIGQUEUE_PREALLOC; __sigqueue_free(first); __sigqueue_free(q); yue.tao - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists