From d62c6f2df5c0e1390b9a1f45b1b52689e3f234f0 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 27 Apr 2026 07:30:35 -0700 Subject: [PATCH 1/4] netconsole: return count instead of strnlen(buf, count) from store callbacks Several configfs store callbacks in netconsole end with: ret = strnlen(buf, count); This under-reports the number of bytes consumed when the input contains an embedded NUL within count, telling the VFS that fewer bytes were written than userspace actually handed in. A conformant partial-write loop would then retry the trailing bytes against a callback that has already accepted them. Every other configfs driver in the tree returns count directly from its store callbacks once parsing has succeeded, including drivers/nvme/target/configfs.c, drivers/gpio/gpio-sim.c, drivers/most/configfs.c, drivers/block/null_blk/main.c, drivers/pci/endpoint/pci-ep-cfs.c, and the rest of the configfs users. netconsole was the outlier (along with drivers/infiniband/core/cma_configfs.c, which has the same latent issue). Align netconsole with the rest of the configfs ecosystem: return count once the parser/validator has accepted the input. The numeric and boolean parsers (kstrtobool, kstrtou16, mac_pton, netpoll_parse_ip_addr) have already validated the meaningful prefix; any trailing bytes are padding and should simply be reported as consumed. Fixes: 0bcc1816188e ("[NET] netconsole: Support dynamic reconfiguration using configfs") Reviewed-by: Simon Horman Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20260427-netconsole_ai_fixes-v2-1-59965f29d9cc@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netconsole.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 205384dab89a..76d7fbf9e188 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -752,7 +752,7 @@ static ssize_t enabled_store(struct config_item *item, unregister_netcons_consoles(); } - ret = strnlen(buf, count); + ret = count; /* Deferred cleanup */ netconsole_process_cleanups(); out_unlock: @@ -781,7 +781,7 @@ static ssize_t release_store(struct config_item *item, const char *buf, nt->release = release; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -807,7 +807,7 @@ static ssize_t extended_store(struct config_item *item, const char *buf, goto out_unlock; nt->extended = extended; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -830,7 +830,7 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf, trim_newline(nt->np.dev_name, IFNAMSIZ); dynamic_netconsole_mutex_unlock(); - return strnlen(buf, count); + return count; } static ssize_t local_port_store(struct config_item *item, const char *buf, @@ -849,7 +849,7 @@ static ssize_t local_port_store(struct config_item *item, const char *buf, ret = kstrtou16(buf, 10, &nt->np.local_port); if (ret < 0) goto out_unlock; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -871,7 +871,7 @@ static ssize_t remote_port_store(struct config_item *item, ret = kstrtou16(buf, 10, &nt->np.remote_port); if (ret < 0) goto out_unlock; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -896,7 +896,7 @@ static ssize_t local_ip_store(struct config_item *item, const char *buf, goto out_unlock; nt->np.ipv6 = !!ipv6; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -921,7 +921,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf, goto out_unlock; nt->np.ipv6 = !!ipv6; - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -957,7 +957,7 @@ static ssize_t remote_mac_store(struct config_item *item, const char *buf, goto out_unlock; memcpy(nt->np.remote_mac, remote_mac, ETH_ALEN); - ret = strnlen(buf, count); + ret = count; out_unlock: dynamic_netconsole_mutex_unlock(); return ret; @@ -1133,7 +1133,7 @@ static ssize_t sysdata_msgid_enabled_store(struct config_item *item, disable_sysdata_feature(nt, SYSDATA_MSGID); unlock_ok: - ret = strnlen(buf, count); + ret = count; dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; @@ -1162,7 +1162,7 @@ static ssize_t sysdata_release_enabled_store(struct config_item *item, disable_sysdata_feature(nt, SYSDATA_RELEASE); unlock_ok: - ret = strnlen(buf, count); + ret = count; dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; @@ -1191,7 +1191,7 @@ static ssize_t sysdata_taskname_enabled_store(struct config_item *item, disable_sysdata_feature(nt, SYSDATA_TASKNAME); unlock_ok: - ret = strnlen(buf, count); + ret = count; dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; @@ -1225,7 +1225,7 @@ static ssize_t sysdata_cpu_nr_enabled_store(struct config_item *item, disable_sysdata_feature(nt, SYSDATA_CPU_NR); unlock_ok: - ret = strnlen(buf, count); + ret = count; dynamic_netconsole_mutex_unlock(); mutex_unlock(&netconsole_subsys.su_mutex); return ret; From e6dd94252b0fa7b4fcc00577c6898432c5d97a08 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 27 Apr 2026 07:30:36 -0700 Subject: [PATCH 2/4] netconsole: avoid clobbering userdatum value on truncated write userdatum_value_store() bounds count by MAX_EXTRADATA_VALUE_LEN (200) and then copies straight into udm->value, which is itself 200 bytes: if (count > MAX_EXTRADATA_VALUE_LEN) return -EMSGSIZE; ... ret = strscpy(udm->value, buf, sizeof(udm->value)); if (ret < 0) goto out_unlock; If userspace writes exactly MAX_EXTRADATA_VALUE_LEN bytes with no NUL within them, strscpy() copies 199 bytes plus a NUL into udm->value and returns -E2BIG. The function jumps to out_unlock and reports the error to userspace, but udm->value has already been overwritten with the truncated string and update_userdata() is skipped, so the corruption is not yet visible on the wire. The next successful write to any userdatum entry under the same target calls update_userdata(), which packs udm->value into the active netconsole payload. From that point on, every netconsole message carries the silently truncated value, and userspace has no indication that a previous, error-returning write left state behind. Tighten the entry check from "count > MAX_EXTRADATA_VALUE_LEN" to "count >= MAX_EXTRADATA_VALUE_LEN". With count strictly less than sizeof(udm->value), strscpy() can no longer return -E2BIG here, so the corrupting truncation path is removed entirely. Fixes: 8a6d5fec6c7f ("net: netconsole: add a userdata config_group member to netconsole_target") Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20260427-netconsole_ai_fixes-v2-2-59965f29d9cc@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netconsole.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 76d7fbf9e188..595e09bd1ccf 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -1076,15 +1076,13 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, struct userdata *ud; ssize_t ret; - if (count > MAX_EXTRADATA_VALUE_LEN) + if (count >= MAX_EXTRADATA_VALUE_LEN) return -EMSGSIZE; mutex_lock(&netconsole_subsys.su_mutex); dynamic_netconsole_mutex_lock(); - - ret = strscpy(udm->value, buf, sizeof(udm->value)); - if (ret < 0) - goto out_unlock; + /* count is bounded above, so strscpy() cannot truncate here */ + strscpy(udm->value, buf, sizeof(udm->value)); trim_newline(udm->value, sizeof(udm->value)); ud = to_userdata(item->ci_parent); From 92ceb7bff62c2606f664c204750eca0b85d44112 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 27 Apr 2026 07:30:37 -0700 Subject: [PATCH 3/4] netconsole: propagate device name truncation in dev_name_store() dev_name_store() calls strscpy(nt->np.dev_name, buf, IFNAMSIZ) without checking the return value. If userspace writes an interface name longer than IFNAMSIZ - 1, strscpy() silently truncates and returns -E2BIG, but the function ignores it and reports a fully successful write back to userspace. If a real interface happens to match the truncated name, netconsole will bind to the wrong device on the next enable, sending kernel logs and panic output to an unintended network segment with no indication to userspace that anything was rewritten. Reject writes whose length cannot fit in nt->np.dev_name up front: if (count >= IFNAMSIZ) return -ENAMETOOLONG; This is not a big deal of a problem, but, it is still the correct approach. Fixes: 0bcc1816188e57 ("[NET] netconsole: Support dynamic reconfiguration using configfs") Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20260427-netconsole_ai_fixes-v2-3-59965f29d9cc@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netconsole.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 595e09bd1ccf..b3b36e3ddd03 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -817,6 +817,13 @@ static ssize_t dev_name_store(struct config_item *item, const char *buf, size_t count) { struct netconsole_target *nt = to_target(item); + size_t len = count; + + /* Account for a trailing newline appended by tools like echo */ + if (len && buf[len - 1] == '\n') + len--; + if (len >= IFNAMSIZ) + return -ENAMETOOLONG; dynamic_netconsole_mutex_lock(); if (nt->state == STATE_ENABLED) { From 869cd6490fafe09c89a15d01610e8a03932d79f0 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 27 Apr 2026 07:30:38 -0700 Subject: [PATCH 4/4] netconsole: restore userdatum value on update_userdata() failure userdatum_value_store() updates udm->value first and only then calls update_userdata() to rebuild the on-the-wire payload. If update_userdata() fails (e.g. -ENOMEM from kmalloc), the function returns the error to userspace, but udm->value already holds the new string while the live nt->userdata buffer still reflects the old one. The next successful write to any sibling userdatum on the same target will call update_userdata() again, which walks every entry and packs the now-stale udm->value into the payload. The failed write is thus silently activated later, with no indication to userspace that the value it tried to set was rejected. Snapshot the previous value before overwriting udm->value and restore it if update_userdata() fails so the visible state and the active payload stay consistent. Fixes: eb83801af2dc ("netconsole: Dynamic allocation of userdata buffer") Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20260427-netconsole_ai_fixes-v2-4-59965f29d9cc@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netconsole.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index b3b36e3ddd03..57dd6821a8aa 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -1079,6 +1079,7 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, size_t count) { struct userdatum *udm = to_userdatum(item); + char old_value[MAX_EXTRADATA_VALUE_LEN]; struct netconsole_target *nt; struct userdata *ud; ssize_t ret; @@ -1088,6 +1089,8 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, mutex_lock(&netconsole_subsys.su_mutex); dynamic_netconsole_mutex_lock(); + /* Snapshot for rollback if update_userdata() fails below */ + strscpy(old_value, udm->value, sizeof(old_value)); /* count is bounded above, so strscpy() cannot truncate here */ strscpy(udm->value, buf, sizeof(udm->value)); trim_newline(udm->value, sizeof(udm->value)); @@ -1095,8 +1098,11 @@ static ssize_t userdatum_value_store(struct config_item *item, const char *buf, ud = to_userdata(item->ci_parent); nt = userdata_to_target(ud); ret = update_userdata(nt); - if (ret < 0) + if (ret < 0) { + /* Restore the previous value so it matches the live payload */ + strscpy(udm->value, old_value, sizeof(udm->value)); goto out_unlock; + } ret = count; out_unlock: dynamic_netconsole_mutex_unlock();