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

Message-ID: <CA+FuTSfzoQ_b4mu-kbXa6Gz5g3ZV4kz+ygLb7x==BJVD_040sQ@mail.gmail.com>
Date:   Sun, 28 Mar 2021 10:08:30 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Kurt Kanzenbach <kurt@...utronix.de>
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 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")

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.