6.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Gustavo Luiz Duarte gustavold@gmail.com
[ Upstream commit d7d2fcf7ae31471b4e08b7e448b8fd0ec2e06a1b ]
There is a race between operations that iterate over the userdata cg_children list and concurrent add/remove of userdata items through configfs. The update_userdata() function iterates over the nt->userdata_group.cg_children list, and count_extradata_entries() also iterates over this same list to count nodes.
Quoting from Documentation/filesystems/configfs.rst:
A subsystem can navigate the cg_children list and the ci_parent pointer to see the tree created by the subsystem. This can race with configfs' management of the hierarchy, so configfs uses the subsystem mutex to protect modifications. Whenever a subsystem wants to navigate the hierarchy, it must do so under the protection of the subsystem mutex.
Without proper locking, if a userdata item is added or removed concurrently while these functions are iterating, the list can be accessed in an inconsistent state. For example, the list_for_each() loop can reach a node that is being removed from the list by list_del_init() which sets the nodes' .next pointer to point to itself, so the loop will never end (or reach the WARN_ON_ONCE in update_userdata() ).
Fix this by holding the configfs subsystem mutex (su_mutex) during all operations that iterate over cg_children. This includes: - userdatum_value_store() which calls update_userdata() to iterate over cg_children - All sysdata_*_enabled_store() functions which call count_extradata_entries() to iterate over cg_children
The su_mutex must be acquired before dynamic_netconsole_mutex to avoid potential lock ordering issues, as configfs operations may already hold su_mutex when calling into our code.
Fixes: df03f830d099 ("net: netconsole: cache userdata formatted string in netconsole_target") Signed-off-by: Gustavo Luiz Duarte gustavold@gmail.com Link: https://patch.msgid.link/20251029-netconsole-fix-warn-v1-1-0d0dd4622f48@gmai... Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/net/netconsole.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index e3722de08ea9f..3ff1dfc3d26b2 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -928,6 +928,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, if (count > MAX_EXTRADATA_VALUE_LEN) return -EMSGSIZE;
+ mutex_lock(&netconsole_subsys.su_mutex); mutex_lock(&dynamic_netconsole_mutex);
ret = strscpy(udm->value, buf, sizeof(udm->value)); @@ -941,6 +942,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, ret = count; out_unlock: mutex_unlock(&dynamic_netconsole_mutex); + mutex_unlock(&netconsole_subsys.su_mutex); return ret; }
@@ -966,6 +968,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item, if (ret) return ret;
+ mutex_lock(&netconsole_subsys.su_mutex); mutex_lock(&dynamic_netconsole_mutex); curr = !!(nt->sysdata_fields & SYSDATA_MSGID); if (msgid_enabled == curr) @@ -986,6 +989,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item, ret = strnlen(buf, count); unlock: mutex_unlock(&dynamic_netconsole_mutex); + mutex_unlock(&netconsole_subsys.su_mutex); return ret; }
@@ -1000,6 +1004,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item, if (ret) return ret;
+ mutex_lock(&netconsole_subsys.su_mutex); mutex_lock(&dynamic_netconsole_mutex); curr = !!(nt->sysdata_fields & SYSDATA_RELEASE); if (release_enabled == curr) @@ -1020,6 +1025,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item, ret = strnlen(buf, count); unlock: mutex_unlock(&dynamic_netconsole_mutex); + mutex_unlock(&netconsole_subsys.su_mutex); return ret; }
@@ -1034,6 +1040,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item, if (ret) return ret;
+ mutex_lock(&netconsole_subsys.su_mutex); mutex_lock(&dynamic_netconsole_mutex); curr = !!(nt->sysdata_fields & SYSDATA_TASKNAME); if (taskname_enabled == curr) @@ -1054,6 +1061,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item, ret = strnlen(buf, count); unlock: mutex_unlock(&dynamic_netconsole_mutex); + mutex_unlock(&netconsole_subsys.su_mutex); return ret; }
@@ -1069,6 +1077,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item, if (ret) return ret;
+ mutex_lock(&netconsole_subsys.su_mutex); mutex_lock(&dynamic_netconsole_mutex); curr = !!(nt->sysdata_fields & SYSDATA_CPU_NR); if (cpu_nr_enabled == curr) @@ -1097,6 +1106,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item, ret = strnlen(buf, count); unlock: mutex_unlock(&dynamic_netconsole_mutex); + mutex_unlock(&netconsole_subsys.su_mutex); return ret; }