netdev - [PATCH net-next v3 0/2] net/bridge: Add skb drop reasons to the most common drop points

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

Message-ID: <20241219163606.717758-1-rrendec@redhat.com>
Date: Thu, 19 Dec 2024 11:36:04 -0500
From: Radu Rendec <rrendec@...hat.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>,
	Ido Schimmel <idosch@...sch.org>,
	Roopa Prabhu <roopa@...dia.com>
Cc: bridge@...ts.linux.dev,
	netdev@...r.kernel.org,
	Simon Horman <horms@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: [PATCH net-next v3 0/2] net/bridge: Add skb drop reasons to the most common drop points

The bridge input code may drop frames for various reasons and at various
points in the ingress handling logic. Currently kfree_skb() is used
everywhere, and therefore no drop reason is specified. Add drop reasons
to the most common drop points.

The purpose of this series is to address the most common drop points on
the bridge ingress path. It does not exhaustively add drop reasons to
the entire bridge code. The intention here is to incrementally add drop
reasons to the rest of the bridge code in follow up patches.

Most of the skb drop points that are addressed in this series can be
easily tested by sending crafted packets. The diagram below shows a
simple test configuration, and some examples using `packit`(*) are
also included. The bridge is set up with STP disabled.
(*) https://github.com/resurrecting-open-source-projects/packit

The following changes were *not* tested:
* SKB_DROP_REASON_NOMEM in br_flood(). It's not easy to trigger an OOM
  condition for testing purposes, while everything else works correctly.
* All drop reasons in br_multicast_flood(). I could not find an easy way
  to make a crafted packet get there.
* SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE in br_handle_frame_finish()
  when the port state is BR_STATE_DISABLED, because in that case the
  frame is already dropped in the switch/case block at the end of
  br_handle_frame().

    +-------+
    |  br0  |
    +---+---+
        |
    +---+---+  veth pair  +-------+
    | veth0 +-------------+ xeth0 |
    +-------+             +-------+

SKB_DROP_REASON_MAC_INVALID_SOURCE - br_handle_frame()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
  -e 01:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
  -p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL - br_handle_frame()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
  -e 02:22:33:44:55:66 -E 01:80:c2:00:00:01 -c 1 \
  -p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE - br_handle_frame()
bridge link set dev veth0 state 0 # disabled
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
  -e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
  -p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE - br_handle_frame_finish()
bridge link set dev veth0 state 2 # learning
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
  -e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
  -p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_NO_TX_TARGET - br_flood()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
  -e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
  -p '0x de ad be ef' -i xeth0

Changes since v2:
- Minor update to the description of SKB_DROP_REASON_NO_TX_TARGET.
- Do not trigger a warning in br_flood() and br_multicast_flood() if
  maybe_deliver() returns a different error than -ENOMEM.
- Keep the error path consolidated in br_handle_frame_finish() and
  br_handle_frame(), and use a `reason` variable.

Changes since v1:
- Add patch #1, which makes it possible to reuse the existing
  SKB_DROP_REASON_VXLAN_NO_REMOTE drop reason from vxlan in the bridge
  module, where a similar drop case exists.
- Don't add SKB_DROP_REASON_BRIDGE_NO_EGRESS_PORT. Instead, use either
  SKB_DROP_REASON_NO_TX_TARGET or SKB_DROP_REASON_NOMEM, depending on
  the case.
- For better clarity, rename SKB_DROP_REASON_BRIDGE_INGRESS_PORT_NFWD to
  SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE.

Radu Rendec (2):
  net: vxlan: rename SKB_DROP_REASON_VXLAN_NO_REMOTE
  net: bridge: add skb drop reasons to the most common drop points

 drivers/net/vxlan/vxlan_core.c |  4 ++--
 drivers/net/vxlan/vxlan_mdb.c  |  2 +-
 include/net/dropreason-core.h  | 18 +++++++++++++++---
 net/bridge/br_forward.c        | 16 ++++++++++++----
 net/bridge/br_input.c          | 20 +++++++++++++++-----
 5 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.47.1