Skip to content

Commit d7d2fcf

Browse files
gustavoldkuba-moo
authored andcommitted
netconsole: Acquire su_mutex before navigating configs hierarchy
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: df03f83 ("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@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent c211f5d commit d7d2fcf

File tree

1 file changed

+10
-0
lines changed

1 file changed

+10
-0
lines changed

drivers/net/netconsole.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf,
936936
if (count > MAX_EXTRADATA_VALUE_LEN)
937937
return -EMSGSIZE;
938938

939+
mutex_lock(&netconsole_subsys.su_mutex);
939940
mutex_lock(&dynamic_netconsole_mutex);
940941

941942
ret = strscpy(udm->value, buf, sizeof(udm->value));
@@ -949,6 +950,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf,
949950
ret = count;
950951
out_unlock:
951952
mutex_unlock(&dynamic_netconsole_mutex);
953+
mutex_unlock(&netconsole_subsys.su_mutex);
952954
return ret;
953955
}
954956

@@ -974,6 +976,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item,
974976
if (ret)
975977
return ret;
976978

979+
mutex_lock(&netconsole_subsys.su_mutex);
977980
mutex_lock(&dynamic_netconsole_mutex);
978981
curr = !!(nt->sysdata_fields & SYSDATA_MSGID);
979982
if (msgid_enabled == curr)
@@ -994,6 +997,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item,
994997
ret = strnlen(buf, count);
995998
unlock:
996999
mutex_unlock(&dynamic_netconsole_mutex);
1000+
mutex_unlock(&netconsole_subsys.su_mutex);
9971001
return ret;
9981002
}
9991003

@@ -1008,6 +1012,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item,
10081012
if (ret)
10091013
return ret;
10101014

1015+
mutex_lock(&netconsole_subsys.su_mutex);
10111016
mutex_lock(&dynamic_netconsole_mutex);
10121017
curr = !!(nt->sysdata_fields & SYSDATA_RELEASE);
10131018
if (release_enabled == curr)
@@ -1028,6 +1033,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item,
10281033
ret = strnlen(buf, count);
10291034
unlock:
10301035
mutex_unlock(&dynamic_netconsole_mutex);
1036+
mutex_unlock(&netconsole_subsys.su_mutex);
10311037
return ret;
10321038
}
10331039

@@ -1042,6 +1048,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item,
10421048
if (ret)
10431049
return ret;
10441050

1051+
mutex_lock(&netconsole_subsys.su_mutex);
10451052
mutex_lock(&dynamic_netconsole_mutex);
10461053
curr = !!(nt->sysdata_fields & SYSDATA_TASKNAME);
10471054
if (taskname_enabled == curr)
@@ -1062,6 +1069,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item,
10621069
ret = strnlen(buf, count);
10631070
unlock:
10641071
mutex_unlock(&dynamic_netconsole_mutex);
1072+
mutex_unlock(&netconsole_subsys.su_mutex);
10651073
return ret;
10661074
}
10671075

@@ -1077,6 +1085,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item,
10771085
if (ret)
10781086
return ret;
10791087

1088+
mutex_lock(&netconsole_subsys.su_mutex);
10801089
mutex_lock(&dynamic_netconsole_mutex);
10811090
curr = !!(nt->sysdata_fields & SYSDATA_CPU_NR);
10821091
if (cpu_nr_enabled == curr)
@@ -1105,6 +1114,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item,
11051114
ret = strnlen(buf, count);
11061115
unlock:
11071116
mutex_unlock(&dynamic_netconsole_mutex);
1117+
mutex_unlock(&netconsole_subsys.su_mutex);
11081118
return ret;
11091119
}
11101120

0 commit comments

Comments
 (0)