netdev - Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start

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: <20110719202922.GA2352@minipsycho>
Date:	Tue, 19 Jul 2011 22:29:23 +0200
From:	Jiri Pirko <jpirko@...hat.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org,
	Alexey Dobriyan <adobriyan@...il.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in
 ndo_start_xmit methods

Tue, Jul 19, 2011 at 10:02:18PM CEST, eric.dumazet@...il.com wrote:
>Le mardi 19 juillet 2011 à 15:52 -0400, Neil Horman a écrit :
>> This oops was reported recently when running a pktgen script that called for a
>> transmitted skb to be cloned and sent 100 times using the clone_skb command to a
>> bridge device with several attached interface:
>> 
>...
>
>> Turns out the pktgen driver doesn't actually clone skbs, but rather shares them,
>> increasing the reference count of the skb for each send operation.  This works
>> for most drivers because most drivers don't store or care about any state in the
>> skb itself, but several do.  For instance, the above tun/tap driver and other
>> soft drivers (vlans, bonding/bridging), all requeue frames to a physical device,
>> meaning the skb next and prev pointers will be set.  Other drivers also care
>> about skb state.  The virtio_net driver for instance uses the skb->cb space to
>> store a vnet header and several converged adapters adjust the data pointer of an
>> skb to prepend a device control header to the skb.  Drivers expect skbs
>> submitted for i/o to be in their control and unshared with other users, an
>> assumption which pktgen is violating, the result being multiple skb users
>> corrupting one antohers state and producing oopses like the one above.  The
>> solution is to make pktgen clone the skb for each transmit so as to ensure the
>> drivers assumptions about private exclusive access to the skb is maintained.
>> 
>> Tested successfully by myself
>> Signed-off-by: Neil Horman <nhorman@...driver.com>
>> Reported-by: Jiri Pirko <jpirko@...hat.com>
>> CC: Eric Dumazet <eric.dumazet@...il.com>
>> CC: Alexey Dobriyan <adobriyan@...il.com>
>> CC: David S. Miller <davem@...emloft.net>
>> ---
>
>This will kill pktgen performance ?
>
>pktgen is only for sysadmins, and very skilled ones :)

You are right, but it may not cause panic, right? In case this patch
would cause significant performance regression, how about to just forbid
pktgen to run on soft-netdevs ?

Jirka

>
>BTW you forgot to CC pktgen author, Robert Olsson
>
>
>
>--
>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
--
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