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: Mon, 13 Jan 2014 09:55:18 +0000 From: David Laight <David.Laight@...LAB.COM> To: 'Cong Wang' <cwang@...pensource.com>, Daniel Borkmann <dborkman@...hat.com> CC: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org> Subject: RE: [PATCH net-next 3/3] packet: use percpu mmap tx frame pending refcount From: Cong Wang > On Sun, Jan 12, 2014 at 8:22 AM, Daniel Borkmann <dborkman@...hat.com> wrote: > > +static void packet_inc_pending(struct packet_ring_buffer *rb) > > +{ > > + this_cpu_inc(*rb->pending_refcnt); > > +} > > + > > +static void packet_dec_pending(struct packet_ring_buffer *rb) > > +{ > > + this_cpu_dec(*rb->pending_refcnt); > > +} > > + > > +static int packet_read_pending(const struct packet_ring_buffer *rb) > > +{ > > + int i, refcnt = 0; > > + > > + /* We don't use pending refcount in rx_ring. */ > > + if (rb->pending_refcnt == NULL) > > + return 0; > > + > > + for_each_possible_cpu(i) > > + refcnt += *per_cpu_ptr(rb->pending_refcnt, i); > > + > > + return refcnt; > > +} > > How is this supposed to work? Since there is no lock, > you can't read accurate refcnt. Take a look at lib/percpu_counter.c. > > I guess for some reason you don't care the accuracy? > Then at least you need to comment in the code. Hmmm... did the code ever work? The value looks like the number of active transmits (s/pending/tx_pending/) The test of the number of pending tx is at the bottom of a code loop, I'd expect that some action should be locked against this check - but even the atomic read doesn't do this. Check what happens if the count reads as 1 just before another cpu decrements it to zero (or reads 0 just before being incremented). In one of those cases something is likely to go wrong. I'm actually surprised that there isn't a mutex covering the code path. David -- 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