netdev - Re: [PATCH net-next v1] flow_dissector: Add support for HSR

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, 03 Mar 2022 09:08:35 +0100
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Alexander Lobakin <alobakin@...me>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Eric Dumazet <edumazet@...gle.com>,
        Paul Blakey <paulb@...dia.com>,
        Yoshiki Komachi <komachi.yoshiki@...il.com>,
        zhang kai <zhangkaiheb@....com>,
        Juhee Kang <claudiajkang@...il.com>,
        Andreas Oetken <ennoerlangen@...il.com>,
        George McCollister <george.mccollister@...il.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        netdev@...r.kernel.org,
        Anthony Harivel <anthony.harivel@...utronix.de>
Subject: Re: [PATCH net-next v1] flow_dissector: Add support for HSR

Hi Jakub,

On Wed Mar 02 2022, Jakub Kicinski wrote:
> On Mon, 28 Feb 2022 20:58:56 +0100 Kurt Kanzenbach wrote:
>> Network drivers such as igb or igc call eth_get_headlen() to determine the
>> header length for their to be constructed skbs in receive path.
>> 
>> When running HSR on top of these drivers, it results in triggering BUG_ON() in
>> skb_pull(). The reason is the skb headlen is not sufficient for HSR to work
>> correctly. skb_pull() notices that.
>
> Should that also be fixed? BUG_ON() seems pretty drastic.

It's this statement here:

 https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/include/linux/skbuff.h#n2483

I tried to look up, why is this a BUG_ON() in Thomas' history tree
[1]. Couldn't find an explanation. It's been introduced by this commit:

|commit 1a0153507ffae9cf3350e76c12d441788c0191e1 (HEAD)
|Author: Linus Torvalds <torvalds@...lon.transmeta.com>
|Date:   Mon Feb 4 18:11:38 2002 -0800
|
|    v2.4.3.2 -> v2.4.3.3
|    
|      - Hui-Fen Hsu: sis900 driver update
|      - NIIBE Yutaka: Super-H update
|      - Alan Cox: more resyncs (ARM down, but more to go)
|      - David Miller: network zerocopy, Sparc sync, qlogic,FC fix, etc.
|      - David Miller/me: get rid of various drivers hacks to do mmap
|      alignment behind the back of the VM layer. Create a real
|      protocol for it.

It seems like BUG/BUG_ON() is the error handling practice in case of
unavailable memory. Even though most functions such as skb_push() or
skb_put() use asserts or skb_over_panic() which also result in BUG() at
the end.

Thanks,
Kurt

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/

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