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: Tue, 12 Aug 2008 06:36:22 +0000 From: Jarek Poplawski <jarkao2@...il.com> To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> Cc: David Miller <davem@...emloft.net>, emil.s.tantilov@...el.com, jeffrey.t.kirsher@...el.com, netdev@...r.kernel.org Subject: Re: [BUG] NULL pointer dereference in skb_dequeue On Mon, Aug 11, 2008 at 04:26:57PM -0700, Paul E. McKenney wrote: > On Mon, Aug 11, 2008 at 10:01:26AM +0000, Jarek Poplawski wrote: > > On 10-08-2008 21:04, Jarek Poplawski wrote: > > ... > > > Hmm.. Actually, it's completely unreasonable. Let's forget this. > > > > But accidentally it might even sometimes work here... > > > > Currently, the most suspicious place to me seems to be > > __netif_schedule(). Is it legal to store RCU protected pointers out of > > rcu_read_lock() sections? > > Yes, but: > > 1. You need to use one of the update-side primitives to do the > store: rcu_assign_pointer(), list_add_rcu(), etc. > > 2. There has to be some way for multiple updaters to coordinate, > for example: > > a. Only a single task is permitted to update. > > b. Locking is used to coordinate among multiple updaters > (so that only one such updater may proceed at a given > time). > > c. Atomic operations are used to coordinate multiple > updaters. Here be dragons, go for (a) or (b) > instead unless you have an extremely good reason > -and- you have both a proof of correctness and > a totally brutal and malign test suite. > > The main reason to update RCU-protected pointers within rcu_read_lock() > regions is when sharing code between RCU readers and updaters, or when > an RCU reader can see the need to do an update. Sure, but I'm concerned here with pure RCU reading: >From net/sched/sch_generic.c: void __qdisc_run(struct Qdisc *q) { unsigned long start_time = jiffies; while (qdisc_restart(q)) { /* * Postpone processing if * 1. another process needs the CPU; * 2. we've been doing it for too long. */ if (need_resched() || jiffies != start_time) { __netif_schedule(q); This function is run from dev_queue_xmit() (net/core/dev.c) under rcu_read_lock_bh(), and this "q" pointer is passed here for later use (reading) by softirq run net_tx_action(). Alas in net/ RCU primitives are probably omitted in a few places... Thanks for the explanation, Jarek P. break; } } clear_bit(__QDISC_STATE_RUNNING, &q->state); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists