netdev - Re: [PATCH] net: do not pass vlan pkts to real dev pkt handler also

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: <4EE8FE86.90009@gmail.com>
Date:	Wed, 14 Dec 2011 20:52:38 +0100
From:	Nicolas de Pesloüan 
	<nicolas.2p.debian@...il.com>
To:	Jiri Pirko <jpirko@...hat.com>
CC:	Vasu Dev <vasu.dev@...ux.intel.com>, Vasu Dev <vasu.dev@...el.com>,
	netdev@...r.kernel.org, devel@...n-fcoe.org, eric.dumazet@...il.com
Subject: Re: [PATCH] net: do not pass vlan pkts to real dev pkt handler also

Le 13/12/2011 22:45, Jiri Pirko a écrit :
> Tue, Dec 13, 2011 at 06:11:03PM CET, vasu.dev@...ux.intel.com wrote:
>> On Tue, 2011-12-13 at 15:21 +0100, Jiri Pirko wrote:
>>> Tue, Dec 13, 2011 at 02:08:52AM CET, vasu.dev@...ux.intel.com wrote:
>>>> On Mon, 2011-12-12 at 23:56 +0100, Jiri Pirko wrote:
>>>>> Mon, Dec 12, 2011 at 11:19:23PM CET, vasu.dev@...el.com wrote:
>>>>>> The orig_dev has to be updated before going another round
>>>>>> for vlan pkts, otherwise currently unmodified real orig_dev
>>>>>> causes vlan pkt delivered to real orig_dev also.
>>>>>>
>>>>>> The fcoe stack doesn't expects its vlan pkts on real dev
>>>>>> and it causes crash in fcoe stack.
>>>>>
>>>>> Could you please provide more info on where exactly it would crash and
>>>>> why?
>>>>
>>>> Its in fcoe stack due to its fip rx skb list getting corrupt as same skb
>>>> instance getting queued twice without being cloned, though list was well
>>>> protected by its spin lock, it was queued on its two fcoe instances, one
>>>> on real dev and other on its vlan.
>>>>
>>>> I could also handle this gracefully in fcoe stack by cloning but any
>>>> case netdev should not forward vlan pkt to its read dev pkt handler also
>>>> and that is getting fixed with this patch, so patch will restore
>>>> orig_dev uses for *only* vlan pkts as it was with recursive
>>>> __netif_receive_skb calling prior to commit 0dfe178.
>>>
>>>
>>> I do not see into fcoe code, but wouldn't it be good to do skb
>>> skb_share_check in fcoe_ctlr_recv? I suppose that would solve your
>>> problem and looks legal to me.
>>>
>>
>> Yes that will fix along with dropping vlan pkts on real dev, so some
>> additional checking for dropping also. In fact that is what I meant in
>> my last response by "I could also handle this gracefully in fcoe stack
>> by cloning" as skb_share_check() does that conditionally.
>>
>> But as far as this patch goes, are you okay with the fix to not forward
>> vlan pkt on real dev pkt handler ? I think this is required regardless
>> of fcoe stack fixing for shared skb since otherwise all upper layers of
>> real dev pkt handler has to handle with un-expected vlan pkts also.
>
> I think that's what orig_dev is destined for. To provide a possiblility
> to do this. I would like to leave that as it is.

I agree with Jiri.

If a protocol handler is registered on a particular device (instead of NULL), then the handler will 
receive whatever is received on this device. This is true for bridge, for bonding and probably for 
all other "stackable" devices. I don't see any reason to handle it in a different way for vlan.

	Nicolas.

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