netdev - Re: [PATCH net-next] net/packet: Reset MAC header for direct packet transmission

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] [day] [month] [year] [list]

Message-ID: <87blb21lsu.fsf@kurt>
Date:   Mon, 29 Mar 2021 09:06:09 +0200
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net/packet: Reset MAC header for direct packet transmission

On Sun Mar 28 2021, Willem de Bruijn wrote:
> On Fri, Mar 26, 2021 at 11:49 AM Kurt Kanzenbach <kurt@...utronix.de> wrote:
>>
>> Reset MAC header in case of using packet_direct_xmit(), e.g. by specifying
>> PACKET_QDISC_BYPASS. This is needed, because other code such as the HSR layer
>> expects the MAC header to be correctly set.
>>
>> This has been observed using the following setup:
>>
>> |$ ip link add name hsr0 type hsr slave1 lan0 slave2 lan1 supervision 45 version 1
>> |$ ifconfig hsr0 up
>> |$ ./test hsr0
>>
>> The test binary is using mmap'ed sockets and is specifying the
>> PACKET_QDISC_BYPASS socket option.
>>
>> This patch resolves the following warning on a non-patched kernel:
>>
>> |[  112.725394] ------------[ cut here ]------------
>> |[  112.731418] WARNING: CPU: 1 PID: 257 at net/hsr/hsr_forward.c:560 hsr_forward_skb+0x484/0x568
>> |[  112.739962] net/hsr/hsr_forward.c:560: Malformed frame (port_src hsr0)
>>
>> The MAC header is also reset unconditionally in case of PACKET_QDISC_BYPASS is
>> not used.
>
> At the top of __dev_queue_xmit.
>
> I think it is reasonable to expect the mac header to be set in
> ndo_start_xmit. Not sure which other devices besides hsr truly
> requires it.
>
>> Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
>
> If this fixes a bug, it should target net.
>
> Fixes: d346a3fae3ff ("packet: introduce PACKET_QDISC_BYPASS socket
> option")

OK.

>
> This change belongs in __dev_direct_xmit unless all callers except
> packet_direct_xmit do correctly set the mac header. xsk_generic_xmit
> appears to miss it, too, so would equally trigger this warning.

It seems like there are only two callers and the XDP part is missing it
as well. I'll move it to __dev_direct_xmit().

Thanks,
Kurt

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)