netdev - Re: changing dev->needed_headroom/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]

Message-ID: <20130820092013.6ca909e2@nehalam.linuxnetplumber.net>
Date:	Tue, 20 Aug 2013 09:20:13 -0700
From:	Stephen Hemminger <stephen@...workplumber.org>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	Johannes Berg <johannes@...solutions.net>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Ben Hutchings <bhutchings@...arflare.com>,
	netdev <netdev@...r.kernel.org>, linux-wireless@...r.kernel.org
Subject: Re: changing dev->needed_headroom/needed_tailroom?

On Tue, 20 Aug 2013 11:00:18 +0100
Florian Fainelli <f.fainelli@...il.com> wrote:

> 2013/8/5 Johannes Berg <johannes@...solutions.net>:
> > On Fri, 2013-08-02 at 06:11 -0700, Eric Dumazet wrote:
> >> On Fri, 2013-08-02 at 10:55 +0200, Ben Hutchings wrote:
> >>
> >> > I don't think this is safe when the interface is running (even if
> >> > carrier is off).  Some functions may read dev->needed_headroom twice and
> >> > rely on getting the same value each time.
> >>
> >> It should be no problem. Remaining unsafe places should be fixed.
> >
> > Most interesting would be stack devs, which I hadn't even considered. In
> > any case, since I can't completely _rely_ on it, it's an optimisation,
> > the only bugs would be around the double-access and then running
> > over/under the SKB or so?
> 
> As far as I could test this with an Ethernet driver which adjusted its
> needed_headroom by 64 bytes whenever some hardware feature was
> enabled/disabled, this expectedly broke bridge and vlans at least.
> Bridge code does not use the slave ports needed_headroom values, and
> VLAN devices get the parent device needed_headroom only when creating
> the vlan device. The good thing is since the needed_headroom space you
> need is most likely fixed for a given configuration type, you should
> see a pretty "stable" corruption of your SKB head.

All code must check for needed headroom first, and copy packet
if space is not available. Since excess headroom is always safe,
most devices just always use the same worst case headroom.

Even with your changes this will still be necessary since packets will
be still in flight while features change.

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