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, 25 Jul 2008 11:37:57 +0000 From: Jarek Poplawski <jarkao2@...il.com> To: David Miller <davem@...emloft.net> Cc: jussi.kivilinna@...et.fi, kaber@...sh.net, netdev@...r.kernel.org Subject: Re: [PATCH v2 2/3] net_sched: Add accessor function for packet length for qdiscs On Fri, Jul 25, 2008 at 03:57:12AM -0700, David Miller wrote: > From: Jarek Poplawski <jarkao2@...il.com> > Date: Fri, 25 Jul 2008 10:57:48 +0000 > > > On 20-07-2008 01:35, Jussi Kivilinna wrote: > > > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@...et.fi> > > ... > > > diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c > > > index 1afe3ee..f1d2f8e 100644 > > > --- a/net/sched/sch_cbq.c > > > +++ b/net/sched/sch_cbq.c > > > @@ -370,7 +370,6 @@ static int > > > cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > > > { > > > struct cbq_sched_data *q = qdisc_priv(sch); > > > - int len = skb->len; > > > int uninitialized_var(ret); > > > struct cbq_class *cl = cbq_classify(skb, sch, &ret); > > > > > > @@ -391,7 +390,7 @@ cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch) > > > if (ret == NET_XMIT_SUCCESS) { > > > sch->q.qlen++; > > > sch->bstats.packets++; > > > - sch->bstats.bytes+=len; > > > + sch->bstats.bytes += qdisc_pkt_len(skb); > > > > Alas I didn't manage to read this earlier, but this type of changes > > here and elsewhere in this patch looks wrong to me: we shouldn't > > use skb pointer after ->enqueue(). > > It should be OK here, we have the root qdisc locked, so nobody > can remove it from the queue and if we got NET_XMIT_SUCCESS that > thing must not have been freed. > > And anyways, we can't know the qdisc_pkt_len() until the enqueue > fucntion has been called. OK, I see htb did this earlier with ".packets", so it looks like I'm wrong with this - sorry! (Anyway, it seems to unnecessarily restrict the way qdisc are working.) > Even TCP depends upon that NET_XMIT_* return value having some real > meaning, remember the HTB bug we tracked down last week? :-) Do you mean this bug you've forgotten to fix yet? Thanks, Jarek P. -- 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