netdev - Re: skb->frag_list != NULL in start_xmit for device without NETIF_F

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:	Thu, 3 Mar 2011 15:31:41 -0800
From:	Jesse Gross <jesse@...ira.com>
To:	Ian Campbell <Ian.Campbell@...citrix.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Paul Durrant <Paul.Durrant@...rix.com>
Subject: Re: skb->frag_list != NULL in start_xmit for device without NETIF_F_FRAGLIST

On Wed, Mar 2, 2011 at 5:12 AM, Ian Campbell <Ian.Campbell@...citrix.com> wrote:
>> > In particular I'm not sure where the frag_list is supposed to get
>> > resolved in the case where dev_hard_start_xmit takes the netif_needs_gso
>> > == true path and calls dev_gso_segment rather than taking the
>> > __skb_linearize path (the issue appears to occur only when the netback
>> > device has NETIF_F_GSO but not NETIF_F_TSO). AFAICT dev_gso_segment goes
>> > to tcp_tso_segment and then to skb_segment which does appear to create
>> > skbs with a frag_list (although it's not outside the realms of
>> > possibility that I'm reading it wrong).
>>
>> I'm fairly certain that the problem is in skb_segment().  It's not the
>> most tolerant of skbs with frag_lists that do not line up with the
>> requested mss.  Depending on how the original skb is laid out,
>> sometimes this will trigger a BUG_ON and sometimes it creates new
>> frag_lists.  Since there are no further checks after GSO, the skb with
>> a frag_list will get passed to the driver, even if it is not
>> supported.
>
> I wondered if that was the case, but skb_segment made my head spin a
> bit ;-)
>
> Do you think there might be a case for adding a some skb_needs_linearize
> and __skb_linearize calls on the netif_needs_gso paths too, perhaps with
> some WARN_ON's?

Definitely something needs to be done here, as drivers clearly should
not be handed skbs with features that they don't support.  I'm not
sure what the best way to do that is though.  I would like it if
skb_segment() could be made more tolerant of frag_lists in general,
including the cases that currently have BUG_ON.

Currently the function tries to generate packets that are exactly MSS
size (except for the last one).  If the frag_lists don't align with
this we either get BUG or a new skb with a frag_list.  However, the
MSS is simply a maximum - it's also valid to generate packets smaller
than it.  If we did that we would probably be able to handle all of
these cases without doing any linearization.  It's possible that in
some cases it would be faster to do the linearization instead of
generating additional packets but on the other hand the situations
that this affects aren't handled correctly at all right now, so they
probably don't come up too often.

>
> The first check in skb_needs_linearize looks at skb->data_len, I thought
> skb->data_len only covered the paged fragments but the conditions in
> skb_needs_linearize seem to imply that it covers the frag_list length
> too? Was I simply mistaken?

skb->data_len includes both the paged fragments and frag_list.
--
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