netdev - Re: [RFC v2] mac80211: assign needed

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 05 May 2008 16:39:16 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	johannes@...solutions.net
Cc:	tomasw@...il.com, linville@...driver.com, netdev@...r.kernel.org,
	linux-wireless@...r.kernel.org, herbert@...dor.apana.org.au
Subject: Re: [RFC v2] mac80211: assign needed_headroom/tailroom for netdevs

From: Johannes Berg <johannes@...solutions.net>
Date: Tue, 06 May 2008 01:23:55 +0200

[ Herbert CC:'d ]

> On Mon, 2008-05-05 at 16:14 -0700, David Miller wrote:
> 
> > If it is cloned, it makes sure the data reference count allows
> > for modification of the buffer.
> 
> Which buffer? skb->data..skb->tail? And isn't that, in mac80211's case,
> at least currently all the buffer anyway?

Right.

For TSO or cases using frag lists of frag page vectors, it gets
more complicated.

Basically, skb_header_cloned()==false means that you can modify
anything covered by skb->data..skb->tail

> Interesting point. I'll have to see when socket filters are run,
> wpa_supplicant could have a tap open I think. Maybe that's why I'm
> seeing so many cloned packets. I think I'll stacktrace skb_clone and
> print that out.

Note that we can even optimize that case even further if we really
have to.

But try to see why the filter used by WPA Supplicant is not effective.
If the filter fails to match, the packet is free'd immediately using
kfree_skb() which should undo clone'age.

Actually, I think I see a potential problem with how the
->dataref bit fields are managed in this situation.  This is
Herbert's creation, so let's ask him :-)

It seems that if we have a TCP packet for which skb_header_cloned()
is false, and this enters dev_hard_start_xmit(), if any network
taps take the packet, we'll never return to skb_header_cloned()==false
state even if every tap immediately kfree_skb(skb)'s the packet.

When we clone, we'll increment only the low 16-bit part of ->dataref.

On free, since skb->nohdr is set, we'll decrement both of (1 <<
SKB_DATAREF_SHIFT) and the low 16-bit counter.

Therefore the packet never becomes re-writable, right?  Is there some
easy way to cure this Herbert?  Apparently having a tap registered
constantly merely to watch for filtered traffic is quite common. :-/

--
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