ACK: [SRU][J]PATCH v2 0/1] CVE-2024-26837

Tue Feb 18 06:22:09 UTC 2025

On Tue, 18 Feb 2025 16:45:33 +1100
Stewart Hore <stewart.hore at canonical.com> wrote:

> On Thu, Feb 13, 2025 at 01:46:15PM +0000, Andrei Gherzan wrote:
> > [Impact]
> >
> > net: bridge: switchdev: Skip MDB replays of deferred events on offload
> >
> > Before this change, generation of the list of MDB events to replay
> > would race against the creation of new group memberships, either from
> > the IGMP/MLD snooping logic or from user configuration.
> >
> > While new memberships are immediately visible to walkers of
> > br->mdb_list, the notification of their existence to switchdev event
> > subscribers is deferred until a later point in time. So if a replay
> > list was generated during a time that overlapped with such a window,
> > it would also contain a replay of the not-yet-delivered event.
> >
> > The driver would thus receive two copies of what the bridge internally
> > considered to be one single event. On destruction of the bridge, only
> > a single membership deletion event was therefore sent. As a
> > consequence of this, drivers which reference count memberships (at
> > least DSA), would be left with orphan groups in their hardware
> > database when the bridge was destroyed.
> >
> > This is only an issue when replaying additions. While deletion events
> > may still be pending on the deferred queue, they will already have
> > been removed from br->mdb_list, so no duplicates can be generated in
> > that scenario.
> >
> > To a user this meant that old group memberships, from a bridge in
> > which a port was previously attached, could be reanimated (in
> > hardware) when the port joined a new bridge, without the new bridge's
> > knowledge.
> >
> > For example, on an mv88e6xxx system, create a snooping bridge and
> > immediately add a port to it:
> >
> >     root at infix-06-0b-00:~$ ip link add dev br0 up type bridge mcast_snooping 1 && \  
> >     > ip link set dev x3 up master br0  
> >
> > And then destroy the bridge:
> >
> >     root at infix-06-0b-00:~$ ip link del dev br0
> >     root at infix-06-0b-00:~$ mvls atu
> >     ADDRESS             FID  STATE      Q  F  0  1  2  3  4  5  6  7  8  9  a
> >     DEV:0 Marvell 88E6393X
> >     33:33:00:00:00:6a     1  static     -  -  0  .  .  .  .  .  .  .  .  .  .
> >     33:33:ff:87:e4:3f     1  static     -  -  0  .  .  .  .  .  .  .  .  .  .
> >     ff:ff:ff:ff:ff:ff     1  static     -  -  0  1  2  3  4  5  6  7  8  9  a
> >     root at infix-06-0b-00:~$
> >
> > The two IPv6 groups remain in the hardware database because the
> > port (x3) is notified of the host's membership twice: once via the
> > original event and once via a replay. Since only a single delete
> > notification is sent, the count remains at 1 when the bridge is
> > destroyed.
> >
> > Then add the same port (or another port belonging to the same hardware
> > domain) to a new bridge, this time with snooping disabled:
> >
> >     root at infix-06-0b-00:~$ ip link add dev br1 up type bridge mcast_snooping 0 && \  
> >     > ip link set dev x3 up master br1  
> >
> > All multicast, including the two IPv6 groups from br0, should now be
> > flooded, according to the policy of br1. But instead the old
> > memberships are still active in the hardware database, causing the
> > switch to only forward traffic to those groups towards the CPU (port
> > 0).
> >
> > Eliminate the race in two steps:
> >
> > 1. Grab the write-side lock of the MDB while generating the replay
> >    list.
> >
> > This prevents new memberships from showing up while we are generating
> > the replay list. But it leaves the scenario in which a deferred event
> > was already generated, but not delivered, before we grabbed the
> > lock. Therefore:
> >
> > 2. Make sure that no deferred version of a replay event is already
> >    enqueued to the switchdev deferred queue, before adding it to the
> >    replay list, when replaying additions.
> >
> > [Fix]
> >
> > oracular: Not affected
> > noble: Not affected
> > jammy: Fixed by backporting dc489f8 from linux-6.8.y
> > 	`br_mdb_queue_one` and `br_mdb_queue_one` were moved out of
> > 	`br_mdb.c` along with other functions and as part of a larger
> > 	consolidation into `br_switchdev.c`, to avoid a considerable
> > 	restructuring backport, the changes were applied on the original
> > 	file, `br_mdb.c`.  This patch also has a dependency on the
> > 	support for differentiating new VLANs from changed ones, but
> > 	given that the dependency is only for the purposes of comparing
> > 	two `switchdev_obj_port_vlan` objects, this patch was adapted to
> > 	ignore the `changed` struct member.
> > focal: Not affected
> > bionic: Not affected
> > xenial: Not affected
> > trusty: Not affected
> >
> > [Test Case]
> >
> > * Compile all supported arch
> > * Boot tested in a VM
> > * Exercised creating and destroying bridge interfaces
> >
> > [Where problems could occur]
> >
> > This fix might affect Ethernet Bridging functionality especially:
> > - switchdev: module provides glue between core networking code and
> >   device drivers
> > - IGMP snooping: forward multicast traffic based on IGMP/MLD traffic
> >   received from each port
> >
> > v2: Fixed the subject as per the SRU bug template.
> >
> > Tobias Waldekranz (1):
> >   net: bridge: switchdev: Skip MDB replays of deferred events on offload
> >
> >  include/net/switchdev.h   |  3 ++
> >  net/bridge/br_mdb.c       | 78 ++++++++++++++++++++++++---------------
> >  net/switchdev/switchdev.c | 72 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 123 insertions(+), 30 deletions(-)
> >
> > --
> > 2.43.0  
> 
> My followup NACK was invalid because the CVE doesn't require a
> buglink. Sorry for my confusion.

The patch was NACKed so needs to be resubmitted. A resend is fine.

...Juerg

> Acked-by: Stewart Hore <stewart.hore at canonical.com>
> 
> > --
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team  
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20250218/aee9bf2b/attachment.sig>