From nobody Fri Sep 25 10:05:06 2026 Received: from mta-65-225.siemens.flowmailer.net (mta-65-225.siemens.flowmailer.net [185.136.65.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5935946985B for ; Mon, 14 Sep 2026 13:24:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.136.65.225 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392265; cv=none; b=KVyQeM2IshfM6a1WTYTexhXn4zfx29wJeXXJDGZrVxPh0rKfp7U4qBLo/bI/q/jXv12P8DQS0jkLgOY6fHuxK+MGEt6bPhRZ0aGLxysBTyenwc90IRuHJCgI5cnUD+03tutjBsD/9243KCbZZTQ/+BygqA2w3Wb/EkHtuCEOkRk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392265; c=relaxed/simple; bh=ZRyRZwHqmaIeKhF8DBZsJx2hVPfAjKqARrJjthV5Ouo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=PbnWIVXJ9hZF1FEKaMJXPbHHTgZA5ciVMHWqZfL2zKdPNeQhxtudYGE7MaRpYallwjYg+DxF5a4jnGmzrjEqeCDLiV8nFfze7FNF4qhBDtZNA4G01yUNB+vvFpEzIfB7Yzd9PXzSOzlLDtHTuQ2mUNx5/Dc692S+1XE/ZSsAvKY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b=rij8mg3r; arc=none smtp.client-ip=185.136.65.225 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b="rij8mg3r" Received: by mta-65-225.siemens.flowmailer.net with ESMTPSA id 202609141324137a05bbb8e9000207ef for ; Mon, 14 Sep 2026 15:24:13 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=fm1; d=siemens.com; i=alexander.sverdlin@siemens.com; h=Date:From:Subject:To:Message-ID:MIME-Version:Content-Type:Content-Transfer-Encoding:Cc:References:In-Reply-To; bh=nU15p1Uo/GMZ1ItptzKGpIg4lZVwwfXFaJxNjP/Pnn0=; b=rij8mg3r5hjwtffRcjdtw7Z4EH6YJKqw1GHu1L4x1o/iM/ZqAJZ6wwFmhOx6KCIgT1uegZ kIzIU/tvzgJP8ZfVGQ1nOp1KGCcEW8N35O1HCrMYFMBC5DS5tdNzj1o8Z7e/aAXpdWqlxKoq CZiTFWybiksJm92V7dGeDAZTr6tuUa+YN2OiPsjINBSDIevgNHus8LnKWB6QrwVOyEFNLii1 Ux9oFQmLLa8KyAL7pPWMUyiwPE2xKSnNL3gKZiN5FUkW/Vx5RH9XUfuRnTXq/kMSR6Xnvwy+ yccHWwEr3bVgZ/BNhelA8+gjcaoECTFlhdqechSu5K3sgEt7Kd6lP1sg==; From: "A. Sverdlin" To: linux-leds@vger.kernel.org Cc: Alexander Sverdlin , Lee Jones , Pavel Machek , Andrew Lunn , "Mike Marciniszyn (Meta)" , Jakub Kicinski , Kees Cook , =?UTF-8?q?Marek=20Beh=C3=BAn?= , Christian Marangi , linux-kernel@vger.kernel.org Subject: [PATCH 1/3] leds: trigger: netdev: hold lock across sysfs_update_group() in device_name_store() Date: Mon, 14 Sep 2026 15:23:50 +0200 Message-ID: <20260914132356.807004-2-alexander.sverdlin@siemens.com> In-Reply-To: <20260914132356.807004-1-alexander.sverdlin@siemens.com> References: <20260914132356.807004-1-alexander.sverdlin@siemens.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Flowmailer-Platform: Siemens Feedback-ID: 519:519-456497:519-21489:flowmailer From: Alexander Sverdlin device_name_store() calls sysfs_update_group() without holding trigger_data->lock. The is_visible callback reads trigger_data->supported_link_modes, and the sysfs group modification itself is not atomic. This races against netdev_trig_notify() in two ways: 1) Concurrent sysfs_update_group() on the same group: CPU 0 (device_name_store) CPU 1 (linkwatch workqueue) ------------------------- --------------------------- set_device_name(): rtnl_lock() mutex_lock(trigger_data->lock) [sets supported_link_modes] mutex_unlock(trigger_data->lock) rtnl_unlock() sysfs_update_group(): remove("link_10") netdev_trig_notify(): mutex_lock(trigger_data->lock) sysfs_update_group(): remove("link_10") [nop] add("link_10") =E2=86=90 CREAT= ED mutex_unlock(trigger_data->lock) add("link_10") <- EEXIST! 2) The netdev_trig_link_speed_visible() reads supported_link_modes without the lock, racing against get_device_state() called from netdev_trig_notify() which modifies supported_link_modes (a multi-word bitmap) under trigger_data->lock. This can cause netdev_trig_link_speed_visible() to observe a partially-updated bitmap. Fix by holding trigger_data->lock around the sysfs_update_group() call in device_name_store(), matching the locking context used by netdev_trig_notify(). Fixes: 06cdca014eca ("leds: trigger: netdev: Display only supported link sp= eed attribute") Signed-off-by: Alexander Sverdlin --- drivers/leds/trigger/ledtrig-netdev.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/l= edtrig-netdev.c index 5b4e92c14dbb4..354d3b640fa53 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -336,8 +336,14 @@ static ssize_t device_name_store(struct device *dev, if (ret < 0) return ret; =20 - /* Refresh link_speed visibility */ + /* + * Refresh link_speed visibility, serialized against netdev_trig_notify() + * which may concurrently call sysfs_update_group() on the same group + * while reading supported_link_modes via netdev_trig_link_speed_visible(= ). + */ + mutex_lock(&trigger_data->lock); sysfs_update_group(&dev->kobj, &netdev_trig_link_speed_attrs_group); + mutex_unlock(&trigger_data->lock); =20 return size; } --=20 2.55.0 From nobody Fri Sep 25 10:05:06 2026 Received: from mta-64-225.siemens.flowmailer.net (mta-64-225.siemens.flowmailer.net [185.136.64.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8BDF346C4D0 for ; Mon, 14 Sep 2026 13:24:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.136.64.225 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392266; cv=none; b=CjZ4D72e5BacnunhlfXHBXrjKGhmkU4x/x6jcvm7PqBV/YsfvKADtL4/EmEPH6OyZPrTr+T7wyjRBbo5bCEN1qFUBg1g2FTRHz+aa8ZGBa4dKao/Ppjf9TnqUUrmZSMMHXI064PHPMUNf5VymhwEvYT6we9PyiiLZmz3EyU85sA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392266; c=relaxed/simple; bh=HF/vcKA7wASTsFXFodiZsG3sqWtBqytFhS1ZbjKiv3A=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=simhoMjHRTLgeP39uW21/1UrjQRFRpitPlTHT5rYGBa8yEuG472Gj22HVWZbf/kk5MdpGLN0XkAMgJ1qV6XD5JztqBSri8JN/JyCIp+PRLef1iLXQ3KpHD06hovxqG1yTUCEZ63uFYECy227BApviYenf7FqajW54x7iGAw31q8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b=LJMQm/cO; arc=none smtp.client-ip=185.136.64.225 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b="LJMQm/cO" Received: by mta-64-225.siemens.flowmailer.net with ESMTPSA id 20260914132413d12d02eb190002071e for ; Mon, 14 Sep 2026 15:24:13 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=fm1; d=siemens.com; i=alexander.sverdlin@siemens.com; h=Date:From:Subject:To:Message-ID:MIME-Version:Content-Type:Content-Transfer-Encoding:Cc:References:In-Reply-To; bh=+ugNVkCh+thTQLmrHDUIY7tRStnooESAPRePSBpl0mM=; b=LJMQm/cOMZCwrFFkRP5GTa6kcWd9Jj0Dv1mopP1qK3DH8YhUKFMRFEep/I7AC/pAh9p5Nk bpKbCfoxgNEdjWuXg3aivoZM2SM/3iPvFAqR7RBntVY9P7B5Xmwl7RGzba3e1Om8HeRfX0Ut Mnyc6brc47G9Jxas1ly9fwWIaZZmumLdEDYPF2M91daNIMNHH5TZ9cdrqdB9r4nRzk3LP7BS KizixWUXehFZhlbiDhdlWglPoBqhXsymG6fE0YJ8c9K+yUaUko5WXbG++bCbz64AbsL40J1W sCh2hRR9AVrBANWOaeqw9km8AbQn6pwwak4bcBdNdLMIDJUa8NdKzpow==; From: "A. Sverdlin" To: linux-leds@vger.kernel.org Cc: Alexander Sverdlin , Lee Jones , Pavel Machek , Andrew Lunn , "Mike Marciniszyn (Meta)" , Jakub Kicinski , Kees Cook , =?UTF-8?q?Marek=20Beh=C3=BAn?= , Christian Marangi , linux-kernel@vger.kernel.org Subject: [PATCH 2/3] leds: trigger: netdev: fix sysfs_update_group() vs led_trigger_set() races Date: Mon, 14 Sep 2026 15:23:51 +0200 Message-ID: <20260914132356.807004-3-alexander.sverdlin@siemens.com> In-Reply-To: <20260914132356.807004-1-alexander.sverdlin@siemens.com> References: <20260914132356.807004-1-alexander.sverdlin@siemens.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Flowmailer-Platform: Siemens Feedback-ID: 519:519-456497:519-21489:flowmailer Content-Type: text/plain; charset="utf-8" From: Alexander Sverdlin The netdev LED trigger calls sysfs_update_group() to refresh link_speed attribute visibility on NETDEV_CHANGE events and device_name writes. The link_speed attribute group was listed in netdev_led_trigger.groups, so it was also created and destroyed by the LED core via device_add_groups() / device_remove_groups() in led_trigger_set(), which runs under trigger_lock(W). These two owners of the same group share no lock and are not serialized against each other. This was observed as a "cannot create duplicate filename" sysfs splat during boot when the netdev trigger is re-armed while PHY link-up events are being delivered concurrently: sysfs: cannot create duplicate filename '...green:lan/link_10' ... led_trigger_set led_trigger_write Race 1: activate() vs NETDEV_CHANGE (confirmed with reproducer) CPU 0 (led_trigger_set) CPU 1 (linkwatch workqueue) ----------------------- --------------------------- [holds trigger_lock(W)] activate(): register_netdevice_notifier() <- returns netdev_trig_notify(NETDEV_CHANGE): sysfs_update_group() creates "link_10" device_add_groups() creates "link_10" <- EEXIST! Race 2: device_remove_groups() vs NETDEV_CHANGE CPU 0 (led_trigger_set) CPU 1 (linkwatch workqueue) ----------------------- --------------------------- [holds trigger_lock(W)] device_remove_groups() removes "link_10" netdev_trig_notify(NETDEV_CHANGE): sysfs_update_group() creates "link_10" <- ORPHANED deactivate() kfree(trigger_data) <- UAF via orphaned sysfs files Serializing the trigger's sysfs_update_group() against the core under trigger_lock is not viable: NETDEV_CHANGE is delivered from linkwatch with rtnl_mutex held, whereas led_trigger_set() holds trigger_lock(W) and then takes rtnl_mutex via activate() -> set_device_name(). Acquiring trigger_lock from the NETDEV_CHANGE handler would invert that order and deadlock. Fix it by construction instead: drop the link_speed group from netdev_led_trigger.groups and let the trigger own its lifecycle. Create it with sysfs_create_group() in activate() (before registering the notifier) and remove it with sysfs_remove_group() in deactivate() (after unregistering the notifier). The LED core therefore never touches this group, so sysfs_update_group() can no longer collide with device_add_groups() / device_remove_groups() -- eliminating both races above without taking trigger_lock, and hence without the rtnl_mutex inversion. The only remaining concurrency -- device_name_store() and netdev_trig_notify() both calling sysfs_update_group() on the now trigger-owned group -- stays serialized by trigger_data->lock. Creating the group before notifier registration and removing it after notifier unregistration guarantees that no sysfs_update_group() can run while the group is being created or destroyed. Fixes: 06cdca014eca ("leds: trigger: netdev: Display only supported link sp= eed attribute") Signed-off-by: Alexander Sverdlin --- drivers/leds/trigger/ledtrig-netdev.c | 49 +++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/l= edtrig-netdev.c index 354d3b640fa53..89b48c522c227 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -337,9 +337,10 @@ static ssize_t device_name_store(struct device *dev, return ret; =20 /* - * Refresh link_speed visibility, serialized against netdev_trig_notify() - * which may concurrently call sysfs_update_group() on the same group - * while reading supported_link_modes via netdev_trig_link_speed_visible(= ). + * Refresh link_speed attribute visibility. This trigger owns the + * group (created in activate(), removed in deactivate()), so it is only + * serialized against a concurrent sysfs_update_group() in + * netdev_trig_notify() by trigger_data->lock. */ mutex_lock(&trigger_data->lock); sysfs_update_group(&dev->kobj, &netdev_trig_link_speed_attrs_group); @@ -610,9 +611,14 @@ static const struct attribute_group netdev_trig_attrs_= group =3D { .attrs =3D netdev_trig_attrs, }; =20 +/* + * The link_speed attribute group is not listed here: it is created and + * destroyed by activate() / deactivate() so that its sysfs_update_group() + * refreshes can never race with the LED core's device_add_groups() / + * device_remove_groups(). + */ static const struct attribute_group *netdev_trig_groups[] =3D { &netdev_trig_attrs_group, - &netdev_trig_link_speed_attrs_group, NULL, }; =20 @@ -660,7 +666,12 @@ static int netdev_trig_notify(struct notifier_block *n= b, fallthrough; case NETDEV_CHANGE: get_device_state(trigger_data); - /* Refresh link_speed visibility */ + /* + * Refresh link_speed attribute visibility. The group is + * owned by this trigger and never touched by the LED core, so + * updating it here under trigger_data->lock cannot race with + * device_add_groups() / device_remove_groups(). + */ if (evt =3D=3D NETDEV_CHANGE) sysfs_update_group(&led_cdev->dev->kobj, &netdev_trig_link_speed_attrs_group); @@ -784,10 +795,29 @@ static int netdev_trig_activate(struct led_classdev *= led_cdev) =20 led_set_trigger_data(led_cdev, trigger_data); =20 + /* + * Own the link_speed attribute group here instead of listing it in + * netdev_led_trigger.groups, so its later sysfs_update_group() refreshes + * cannot race with the LED core's device_add_groups() / + * device_remove_groups(). Create it before registering the notifier so + * that a NETDEV_CHANGE cannot refresh a not-yet-created group. + */ + rc =3D sysfs_create_group(&led_cdev->dev->kobj, + &netdev_trig_link_speed_attrs_group); + if (rc) + goto err_free; + rc =3D register_netdevice_notifier(&trigger_data->notifier); if (rc) - kfree(trigger_data); + goto err_remove_group; =20 + return 0; + +err_remove_group: + sysfs_remove_group(&led_cdev->dev->kobj, + &netdev_trig_link_speed_attrs_group); +err_free: + kfree(trigger_data); return rc; } =20 @@ -797,6 +827,13 @@ static void netdev_trig_deactivate(struct led_classdev= *led_cdev) =20 unregister_netdevice_notifier(&trigger_data->notifier); =20 + /* + * The notifier is gone, so no sysfs_update_group() can run concurrently + * now; tear down the group we created in activate(). + */ + sysfs_remove_group(&led_cdev->dev->kobj, + &netdev_trig_link_speed_attrs_group); + cancel_delayed_work_sync(&trigger_data->work); =20 dev_put(trigger_data->net_dev); --=20 2.55.0 From nobody Fri Sep 25 10:05:06 2026 Received: from mta-65-226.siemens.flowmailer.net (mta-65-226.siemens.flowmailer.net [185.136.65.226]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D59846D2AA for ; Mon, 14 Sep 2026 13:34:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.136.65.226 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392874; cv=none; b=UZyiZ/SliVhWJd6woeqUiXeE3XHc264ZMvJCdVumuZ7UV7LGEY+u2T6WEe0jiv92MTXVgEnvfLqqyYeASJKI2UwjnZRQCx4kppaRc0V/Zxt94CygFru7iN3Q+9d3cQCjhqd7GSFSBD+RjaH6f7lr6hNNfaySB1F7jryEOmNwuXk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789392874; c=relaxed/simple; bh=9HBnHM60dkZoJJ9TZo4Bf/k5iR/6DDKcRlWsX5OeNqs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=TQBh97lHJGf6NYKN5O2VZ++mwEvtlkjntwOxwxkY3lPhQaAGyaPF1hTwc7x+qQRgickn7XPxfOJEuLdyb3vbFK1aXvcT0blfyA0/9WTPbCZDCQvBQUbQx+YgoUMX2OciS5ELgWVEohaG/qhNLouVbQ2KgA1+9F2wCOp89FJOAZU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b=iARvsx0s; arc=none smtp.client-ip=185.136.65.226 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=siemens.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rts-flowmailer.siemens.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=siemens.com header.i=alexander.sverdlin@siemens.com header.b="iARvsx0s" Received: by mta-65-226.siemens.flowmailer.net with ESMTPSA id 2026091413241496a636686c00020708 for ; Mon, 14 Sep 2026 15:24:14 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=fm1; d=siemens.com; i=alexander.sverdlin@siemens.com; h=Date:From:Subject:To:Message-ID:MIME-Version:Content-Type:Content-Transfer-Encoding:Cc:References:In-Reply-To; bh=xYR6ikkqymzGTBgO/+Kq6rjETk1dH+hHoTqbYvuk6lE=; b=iARvsx0s8NXpxh9lQqfgWyYfIYn0azYiR3Lmef0mQ4i5M3+tY21hnuiNx5QB/wnDu4thcy 8QBsH/2Xrmk4Zc/BWtB9ZCB+5upOFulF3qqM/254XNIH3JSACai0vmMXGKN8SE16wE3dA1OV JKN0/APCg0MjN/oc+8CvFhp5zwuhNQrDp6qPr1Jx9+ih9j4D6Zn1cRdc6F9ETE4SJrirkyt3 SQUinFEqSoXHZrO9VcC7MGs4eyiicsyCBpLKaRnKDKbRz2e+GB+AWCt9W8CHGGlC27QkOaYR KysPnQ3ogQSId+9CwlzMk1dlfcFOw5BLwYZ3Yj0bb+EoFZ6w6de28Eiw==; From: "A. Sverdlin" To: linux-leds@vger.kernel.org Cc: Alexander Sverdlin , Lee Jones , Pavel Machek , Andrew Lunn , "Mike Marciniszyn (Meta)" , Jakub Kicinski , Kees Cook , =?UTF-8?q?Marek=20Beh=C3=BAn?= , Christian Marangi , linux-kernel@vger.kernel.org Subject: [PATCH 3/3] leds: trigger: netdev: serialize mode/interval stores with trigger lock Date: Mon, 14 Sep 2026 15:23:52 +0200 Message-ID: <20260914132356.807004-4-alexander.sverdlin@siemens.com> In-Reply-To: <20260914132356.807004-1-alexander.sverdlin@siemens.com> References: <20260914132356.807004-1-alexander.sverdlin@siemens.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Flowmailer-Platform: Siemens Feedback-ID: 519:519-456497:519-21489:flowmailer Content-Type: text/plain; charset="utf-8" From: Alexander Sverdlin netdev_led_attr_store() and interval_store() update the shared trigger_data state and call set_baseline_state() without holding trigger_data->lock, while netdev_trig_notify() and device_name_store() do the same work under that lock. Because the store paths do not take the lock, it provides no mutual exclusion against them. kernfs only serializes writes to the same attribute file, so writes to two different files run concurrently. netdev_led_attr_store() does a non-atomic read-modify-write of ->mode, so one update can be lost: CPU0 (echo 1 > link_10) CPU1 (echo 1 > link_100) ------------------------- -------------------------- mode =3D trigger_data->mode; // 0 mode =3D trigger_data->mode; // 0 set_bit(LINK_10, &mode); set_bit(LINK_100, &mode); trigger_data->mode =3D mode; // LINK_10 trigger_data->mode =3D mode; // LINK_1= 00 /* LINK_10 update lost */ The store path also races with the notifier, which updates the link state under the lock. set_baseline_state() then programs the LED from a half-updated snapshot, and both CPUs drive the same led_cdev at once: CPU0 (echo 1 > link) CPU1 (NETDEV_CHANGE) ------------------------- -------------------------- mutex_lock(&trigger_data->lock); carrier_link_up =3D false; link_speed =3D SPEED_UNKNOWN; trigger_data->mode =3D mode; set_baseline_state(); /* reads carrier_link_up =3D=3D false, link_speed =3D=3D SPEED_UNKNOWN, torn intermediate state */ get_device_state(); // refill set_baseline_state(); mutex_unlock(&trigger_data->lock); ->hw_control is likewise written non-atomically by both the store path and netdev_trig_notify(), and set_baseline_state() branches on it. Hold trigger_data->lock around the read-modify-write of ->mode, the ->hw_control update and set_baseline_state() in both stores, matching the locking already used by device_name_store() and netdev_trig_notify(). In netdev_led_attr_store() use guard(mutex) so the lock is released on every return path; the invalid-combination check is performed on the locked snapshot so a rejected write does not disturb the running configuration. cancel_delayed_work_sync() is called under the lock; this is safe because netdev_trig_work() never acquires trigger_data->lock. Fixes: d5e01266e7f5 ("leds: trigger: netdev: add additional specific link s= peed mode") Signed-off-by: Alexander Sverdlin --- drivers/leds/trigger/ledtrig-netdev.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/l= edtrig-netdev.c index 89b48c522c227..6556f6d28547f 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -389,7 +389,7 @@ static ssize_t netdev_led_attr_store(struct device *dev= , const char *buf, { struct led_netdev_data *trigger_data =3D led_trigger_get_drvdata(dev); struct led_classdev *led_cdev =3D trigger_data->led_cdev; - unsigned long state, mode =3D trigger_data->mode; + unsigned long state, mode; int ret; int bit; =20 @@ -421,6 +421,16 @@ static ssize_t netdev_led_attr_store(struct device *de= v, const char *buf, return -EINVAL; } =20 + /* + * Serialize the read-modify-write of ->mode and the dependent + * ->hw_control update and set_baseline_state() against concurrent + * attribute stores and netdev_trig_notify(). netdev_trig_work() must + * never take this lock, otherwise the cancel_delayed_work_sync() below + * would deadlock. + */ + guard(mutex)(&trigger_data->lock); + + mode =3D trigger_data->mode; if (state) set_bit(bit, &mode); else @@ -510,10 +520,14 @@ static ssize_t interval_store(struct device *dev, =20 /* impose some basic bounds on the timer interval */ if (value >=3D 5 && value <=3D 10000) { + mutex_lock(&trigger_data->lock); + cancel_delayed_work_sync(&trigger_data->work); =20 atomic_set(&trigger_data->interval, msecs_to_jiffies(value)); set_baseline_state(trigger_data); /* resets timer */ + + mutex_unlock(&trigger_data->lock); } =20 return size; --=20 2.55.0