netdev - Re: [PATCH v3] leds: trigger: netdev: refactor dev matching in netdev_trig

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: <20250429115007.12f8af38@kernel.org>
Date: Tue, 29 Apr 2025 11:50:07 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Tobias Junghans <tobias.junghans@...ub.de>
Cc: Lee Jones <lee@...nel.org>, Andrew Lunn <andrew@...n.ch>,
 linux-leds@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v3] leds: trigger: netdev: refactor dev matching in
 netdev_trig_notify()

On Fri, 25 Apr 2025 15:20:45 +0200 Tobias Junghans wrote:
> If there are network devices with the same name in different
> namespaces, ledtrig-netdev gets confused easily and switches between
> these devices whenever there are NETDEV_CHANGENAME/NETDEV_REGISTER
> notifications.  This happens since ledtrig-netdev only checks for
> device name equality regardless of previous associations with another
> network device with the same name.
> 
> Real world example: eth0 is the primary physical network interface and
> ledltrig-netdev is associated with that interface. If now Docker creates
> a virtual Ethernet interface (vethXXXX), moves it to the
> container's net namespace and renames it to eth0, ledtrig-netdev
> switches to this device and the LED no longer blinks for the original
> (physical) network device.
> 
> Fix this by refactoring the conditions under which to return early with
> NOTIFY_DONE inside netdev_trig_notify():
> 
> - For processing NETDEV_REGISTER events, the device name has to match
>   and no association with a net_dev must exist.
> 
> - For processing NETDEV_CHANGENAME events, the associated and notified
>   network device have to match. Alternatively the device name has to
>   match and no association with a net_dev must exist.
> 
> - For all other events, the associated and notified network device have
>   to match.

Could you split this into two patches for ease of review?
First which factors out all the logic related to deciding if the event
needs to be handled, move it to a new helper which takes the relevant
args are turns bool of whether we should return NOTIFY_DONE immediately
or not. And then a second patch which modifies this logic.
Having the refactor squashed with the change makes it harder to review.