netdev - Re: [Patch net] igb: pass the correct maxlen for eth_get

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: <CAHA+R7MMrrBYbQALovxuhcXrbiWjD_XpgohPXxTvNkcd3-R3WA@mail.gmail.com>
Date:	Wed, 22 Apr 2015 14:56:41 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Alexander Duyck <alexander.duyck@...il.com>
Cc:	Cong Wang <xiyou.wangcong@...il.com>,
	netdev <netdev@...r.kernel.org>,
	intel-wired-lan@...ts.osuosl.org,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Subject: Re: [Patch net] igb: pass the correct maxlen for eth_get_headlen()

On Wed, Apr 22, 2015 at 2:42 PM, Alexander Duyck
<alexander.duyck@...il.com> wrote:
> On 04/22/2015 01:33 PM, Cong Wang wrote:
>> First, make sure you don't miss the TSIP case right above:
>>
>> The frag starting pointer and its size are advanced by:
>>
>> skb_frag_size_sub(frag, IGB_TS_HDR_LEN);
>> ...
>> va += IGB_TS_HDR_LEN;
>>
>> even though we unlikely pull header longer than
>> IGB_RX_HDR_LEN - IGB_TS_HDR_LEN either.
>
> So I believe this is a possible bug, one heck of a corner case to get
> into though.  It requires timestamp in packet, size 240 - 256, and a
> malformed header.
>
> The proper fix would probably be to pull the timestamp out of the packet
> before we add it to the frame.  I'll submit a patch to address this.
>


Huh? Doesn't my patch already fix this? skb_frag_size() is always
up to date. Or you mean another different problem?

>>
>> Second, the check you mentioned above is:
>>
>> if ((size <= IGB_RX_HDR_LEN) && !skb_is_nonlinear(skb))
>>
>> skb is nonlinear _after_ the first igb_add_rx_frag(), a second
>> igb_add_rx_frag() is possible since igb_is_non_eop() could
>> return true.
>
> I'm not sure this part makes any sense.  We pull the data out of the
> first fragment always.  If skb_is_nonlinear is set then we should have
> at least 2K - 16B in the case of igb.  We will never have a second
> fragment without at least 2K of data being given in the first.

Apparently my igb knowledge isn't enough to verify this, I just did
logical analysis.
--
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