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: <20110722013718.GA1993@neilslaptop.think-freely.org>
Date:	Thu, 21 Jul 2011 21:37:19 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	David Miller <davem@...emloft.net>
Cc:	greearb@...delatech.com, eric.dumazet@...il.com, jpirko@...hat.com,
	netdev@...r.kernel.org, adobriyan@...il.com,
	robert.olsson@....uu.se
Subject: Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in
 ndo_start_xmit methods

On Thu, Jul 21, 2011 at 05:08:55PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@...driver.com>
> Date: Thu, 21 Jul 2011 19:50:49 -0400
> 
> > I'm happy to go down this route Dave, and agree, its a more solid solution, but
> > I think the problem with it (which Ben may have been alluding to previously) is
> > that pktgen doesn't use dev_queue_xmit or dev_hard_start_xmit to send frames.
> 
> Neil, that's THE WHOLE POINT, and HOW MY IDEA WORKS.
> 
Dave, RELAX! I GET HOW YOUR IDEA SHOULD WORK!  CAPS ARENT NEEDED! :) 

> Pktgen bypasses those functions, so it won't trigger the check and
> therefore won't trigger making a copy of the SKB, since copying isn't
> needed.
> 
But thats part of the problem, we can't just have pktgen bypass those calls
entirely, assuming that the underlying driver will do so.  See the ppp driver as
an example, it does an skb_pull to add a header to the skb, so multiple
iterations of the multi-skb case will result in stacked header inadvertently.
Another example is the virtio_net driver, which uses the skb->cb area to store
information that would get corrupted in the process.  Neither of those would
pass through the dev_hard_start_xmit path and would be left uncovered by this
solution.

We could fix that by:

a) having pktgen call dev_hard_start_xmit for all frames, giving rise to the
performance issue I was bringing up in my last note.

b) augmenting each driver to check for the flag in the skb as you describe,
which in my mind is no better than having a flag in the netdevice informing
pktgen of weather or not it can use a shared skb approach.

> Only layering devices will end up eventually calling into those two
> functions and trigger the "need to copy because this SKB is pktgen
> shared" check.
> 
See above, that doesn't solve the entire problem.

Regards
Neil

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