net/9p/trans_xen.c | 51 +++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 14 deletions(-)
From: Yufan Chen <ericterminal@gmail.com>
xen_9pfs_front_alloc_dataring() tears down resources on failure but
leaves ring fields stale. If xen_9pfs_front_init() later jumps to the
common error path, xen_9pfs_front_free() may touch the same resources
again, causing duplicate/invalid gnttab_end_foreign_access() calls and
potentially dereferencing a freed intf pointer.
Initialize dataring sentinels before allocation, gate teardown on those
sentinels, and clear ref/intf/data/irq immediately after each release.
This keeps cleanup idempotent for partially initialized rings and
prevents repeated teardown during init failure handling.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
net/9p/trans_xen.c | 51 +++++++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 47af5a10e..85b9ebfaa 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -283,25 +283,33 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
cancel_work_sync(&ring->work);
- if (!priv->rings[i].intf)
+ if (!ring->intf)
break;
- if (priv->rings[i].irq > 0)
- unbind_from_irqhandler(priv->rings[i].irq, ring);
- if (priv->rings[i].data.in) {
- for (j = 0;
- j < (1 << priv->rings[i].intf->ring_order);
+ if (ring->irq >= 0) {
+ unbind_from_irqhandler(ring->irq, ring);
+ ring->irq = -1;
+ }
+ if (ring->data.in) {
+ for (j = 0; j < (1 << ring->intf->ring_order);
j++) {
grant_ref_t ref;
- ref = priv->rings[i].intf->ref[j];
+ ref = ring->intf->ref[j];
gnttab_end_foreign_access(ref, NULL);
+ ring->intf->ref[j] = INVALID_GRANT_REF;
}
- free_pages_exact(priv->rings[i].data.in,
- 1UL << (priv->rings[i].intf->ring_order +
- XEN_PAGE_SHIFT));
+ free_pages_exact(ring->data.in,
+ 1UL << (ring->intf->ring_order +
+ XEN_PAGE_SHIFT));
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ }
+ if (ring->ref != INVALID_GRANT_REF) {
+ gnttab_end_foreign_access(ring->ref, NULL);
+ ring->ref = INVALID_GRANT_REF;
}
- gnttab_end_foreign_access(priv->rings[i].ref, NULL);
- free_page((unsigned long)priv->rings[i].intf);
+ free_page((unsigned long)ring->intf);
+ ring->intf = NULL;
}
kfree(priv->rings);
}
@@ -334,6 +342,12 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
int ret = -ENOMEM;
void *bytes = NULL;
+ ring->intf = NULL;
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ ring->ref = INVALID_GRANT_REF;
+ ring->irq = -1;
+
init_waitqueue_head(&ring->wq);
spin_lock_init(&ring->lock);
INIT_WORK(&ring->work, p9_xen_response);
@@ -379,9 +393,18 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
for (i--; i >= 0; i--)
gnttab_end_foreign_access(ring->intf->ref[i], NULL);
free_pages_exact(bytes, 1UL << (order + XEN_PAGE_SHIFT));
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ }
+ if (ring->ref != INVALID_GRANT_REF) {
+ gnttab_end_foreign_access(ring->ref, NULL);
+ ring->ref = INVALID_GRANT_REF;
+ }
+ if (ring->intf) {
+ free_page((unsigned long)ring->intf);
+ ring->intf = NULL;
}
- gnttab_end_foreign_access(ring->ref, NULL);
- free_page((unsigned long)ring->intf);
+ ring->irq = -1;
return ret;
}
--
2.47.3From: Yufan Chen <ericterminal@gmail.com> This series replaces deprecated simple_strto* parsers in net paths with kstrto* helpers and keeps parser behavior strict. v2: Split the original large patch into a 4-patch series for easier review. Added a prerequisite fix for xen_9pfs dataring cleanup idempotency (Patch 1) to ensure safe error handling during the parser transition. Refined the xen_9pfs version parsing logic to use strsep() for better token handling. Patch 1/4 fixes xen_9pfs dataring cleanup idempotency to avoid repeated resource teardown on init error paths. Patch 2/4 switches xen_9pfs backend version parsing to kstrtouint(). Patch 3/4 updates bridge brport_store() to use kstrtoul(). Patch 4/4 updates sunrpc proc_dodebug() to use kstrtouint(). Yufan Chen (4): 9p/trans_xen: make cleanup idempotent after dataring alloc errors 9p/trans_xen: replace simple_strto* with kstrtouint net: bridge: replace deprecated simple_strtoul with kstrtoul sunrpc: sysctl: replace simple_strtol with kstrtouint net/9p/trans_xen.c | 77 +++++++++++++++++++++++++++------------- net/bridge/br_sysfs_if.c | 5 ++- net/sunrpc/sysctl.c | 24 ++++++------- 3 files changed, 67 insertions(+), 39 deletions(-) -- 2.47.3
On Wed, Feb 25, 2026 at 11:38:36AM +0800, Eric-Terminal wrote: > From: Yufan Chen <ericterminal@gmail.com> > > This series replaces deprecated simple_strto* parsers in net paths with > kstrto* helpers and keeps parser behavior strict. > > v2: > Split the original large patch into a 4-patch series for easier review. > Added a prerequisite fix for xen_9pfs dataring cleanup idempotency > (Patch 1) to ensure safe error handling during the parser transition. > Refined the xen_9pfs version parsing logic to use strsep() for better > token handling. > > Patch 1/4 fixes xen_9pfs dataring cleanup idempotency to avoid repeated > resource teardown on init error paths. > Patch 2/4 switches xen_9pfs backend version parsing to kstrtouint(). > Patch 3/4 updates bridge brport_store() to use kstrtoul(). > Patch 4/4 updates sunrpc proc_dodebug() to use kstrtouint(). Hi, Thanks for your patches. I would like to understand what testing has been performed on these patches. With the possible exception of patch 3/4, I feel that unless they are motivated as bug fixes, these changes are too complex to accept without testing. Although the opinions of the relevant maintainers may differ. And as for 3/4, I lean towards falling into the policy regarding clean-ups not being generally accepted unless it is part of other work.
On Sun, Mar 1, 2026 at 11:29 PM Simon Horman <horms@kernel.org> wrote: > Hi, > > Thanks for your patches. > > I would like to understand what testing has been performed on these patches. > > With the possible exception of patch 3/4, I feel that unless they > are motivated as bug fixes, these changes are too complex to accept > without testing. Although the opinions of the relevant maintainers > may differ. > > And as for 3/4, I lean towards falling into the policy regarding > clean-ups not being generally accepted unless it is part of other work. Hi, Thanks for the feedback. I've verified the series using virtme-ng and a userspace mock harness (for the 9p tokenizing logic). Regarding Patch 1: This is primarily a stability fix. I've tested the error paths by forcing init failures on a non-Xen system; dmesg confirms the new sentinel-based cleanup (NULL-ing intf/data and checking INVALID_GRANT_REF) correctly prevents double-frees and Oops during teardown. Regarding the "complexity" in Patch 2: The strsep + kstrtouint approach was chosen for strictness. I've verified it handles cases like "1,,2" (empty tokens) and "1abc" (malformed input) correctly. The latter is now rejected with -EINVAL, whereas simple_strto* would have silently accepted partial values. The same applies to Patches 3 and 4. The migration to kstrto* ensures that sysfs/procfs interfaces now properly validate the entire input string. P.S. I used a translation tool for the message above, so please excuse any awkward wording. Thanks,
Hi Simon and maintainers, Just a gentle ping on this patch series. Following up on my previous email regarding the testing methodology (using virtme-ng, verifying the Oops fix in Patch 1, and confirming the strictness of strsep + kstrtouint). Does the provided testing information address the concerns? Please let me know if there's anything else I should clarify, or if a v3 is needed to move this forward. Thanks, Yufan On Wed, Mar 4, 2026 at 11:28 PM Eric_Terminal <ericterminal@gmail.com> wrote: > > On Sun, Mar 1, 2026 at 11:29 PM Simon Horman <horms@kernel.org> wrote: > > Hi, > > > > Thanks for your patches. > > > > I would like to understand what testing has been performed on these patches. > > > > With the possible exception of patch 3/4, I feel that unless they > > are motivated as bug fixes, these changes are too complex to accept > > without testing. Although the opinions of the relevant maintainers > > may differ. > > > > And as for 3/4, I lean towards falling into the policy regarding > > clean-ups not being generally accepted unless it is part of other work. > > Hi, > > Thanks for the feedback. I've verified the series using virtme-ng and > a userspace mock harness (for the 9p tokenizing logic). > > Regarding Patch 1: This is primarily a stability fix. I've tested the > error paths by forcing init failures on a non-Xen system; dmesg > confirms the new sentinel-based cleanup (NULL-ing intf/data and > checking INVALID_GRANT_REF) correctly prevents double-frees and Oops > during teardown. > > Regarding the "complexity" in Patch 2: The strsep + kstrtouint > approach was chosen for strictness. I've verified it handles cases > like "1,,2" (empty tokens) and "1abc" (malformed input) correctly. The > latter is now rejected with -EINVAL, whereas simple_strto* would have > silently accepted partial values. > > The same applies to Patches 3 and 4. The migration to kstrto* ensures > that sysfs/procfs interfaces now properly validate the entire input > string. > > P.S. I used a translation tool for the message above, so please excuse > any awkward wording. > > Thanks,
Eric_Terminal wrote on Tue, Mar 24, 2026 at 12:27:50PM +0800: > Hi Simon and maintainers, > > Just a gentle ping on this patch series. > > Following up on my previous email regarding the testing methodology > (using virtme-ng, verifying the Oops fix in Patch 1, and confirming > the strictness of strsep + kstrtouint). > > Does the provided testing information address the concerns? Please let > me know if there's anything else I should clarify, or if a v3 is > needed to move this forward. Part of the problem is that you've mixed a semi-complex 9p patch with other net patches, I think it'd be easiser to move forward if you'd split this (I can't take non-9p patches, and I'm not sure the -net tree wants the 9p patches as we're a small world appart sometimes...) For 9p, I can't (easily) test xen, so I'd appreciate if you could resend with Stefano Stabellini <stefano.stabellini@amd.com> in Ccs as he has been looking after this transport; I can have a quick look but don't have the time for a proper review Thanks, -- Dominique Martinet | Asmadeus
From: Yufan Chen <ericterminal@gmail.com>
xen_9pfs_front_alloc_dataring() tears down resources on failure but
leaves ring fields stale. If xen_9pfs_front_init() later jumps to the
common error path, xen_9pfs_front_free() may touch the same resources
again, causing duplicate/invalid gnttab_end_foreign_access() calls and
potentially dereferencing a freed intf pointer.
Initialize dataring sentinels before allocation, gate teardown on those
sentinels, and clear ref/intf/data/irq immediately after each release.
This keeps cleanup idempotent for partially initialized rings and
prevents repeated teardown during init failure handling.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
net/9p/trans_xen.c | 51 +++++++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 47af5a10e..85b9ebfaa 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -283,25 +283,33 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
cancel_work_sync(&ring->work);
- if (!priv->rings[i].intf)
+ if (!ring->intf)
break;
- if (priv->rings[i].irq > 0)
- unbind_from_irqhandler(priv->rings[i].irq, ring);
- if (priv->rings[i].data.in) {
- for (j = 0;
- j < (1 << priv->rings[i].intf->ring_order);
+ if (ring->irq >= 0) {
+ unbind_from_irqhandler(ring->irq, ring);
+ ring->irq = -1;
+ }
+ if (ring->data.in) {
+ for (j = 0; j < (1 << ring->intf->ring_order);
j++) {
grant_ref_t ref;
- ref = priv->rings[i].intf->ref[j];
+ ref = ring->intf->ref[j];
gnttab_end_foreign_access(ref, NULL);
+ ring->intf->ref[j] = INVALID_GRANT_REF;
}
- free_pages_exact(priv->rings[i].data.in,
- 1UL << (priv->rings[i].intf->ring_order +
- XEN_PAGE_SHIFT));
+ free_pages_exact(ring->data.in,
+ 1UL << (ring->intf->ring_order +
+ XEN_PAGE_SHIFT));
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ }
+ if (ring->ref != INVALID_GRANT_REF) {
+ gnttab_end_foreign_access(ring->ref, NULL);
+ ring->ref = INVALID_GRANT_REF;
}
- gnttab_end_foreign_access(priv->rings[i].ref, NULL);
- free_page((unsigned long)priv->rings[i].intf);
+ free_page((unsigned long)ring->intf);
+ ring->intf = NULL;
}
kfree(priv->rings);
}
@@ -334,6 +342,12 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
int ret = -ENOMEM;
void *bytes = NULL;
+ ring->intf = NULL;
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ ring->ref = INVALID_GRANT_REF;
+ ring->irq = -1;
+
init_waitqueue_head(&ring->wq);
spin_lock_init(&ring->lock);
INIT_WORK(&ring->work, p9_xen_response);
@@ -379,9 +393,18 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
for (i--; i >= 0; i--)
gnttab_end_foreign_access(ring->intf->ref[i], NULL);
free_pages_exact(bytes, 1UL << (order + XEN_PAGE_SHIFT));
+ ring->data.in = NULL;
+ ring->data.out = NULL;
+ }
+ if (ring->ref != INVALID_GRANT_REF) {
+ gnttab_end_foreign_access(ring->ref, NULL);
+ ring->ref = INVALID_GRANT_REF;
+ }
+ if (ring->intf) {
+ free_page((unsigned long)ring->intf);
+ ring->intf = NULL;
}
- gnttab_end_foreign_access(ring->ref, NULL);
- free_page((unsigned long)ring->intf);
+ ring->irq = -1;
return ret;
}
--
2.47.3From: Yufan Chen <ericterminal@gmail.com>
In xen_9pfs_front_init(), parse the backend version list as comma-separated
tokens with kstrtouint(). This improves error reporting and ensures strict
token validation while explicitly requiring protocol version 1.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
net/9p/trans_xen.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 85b9ebfaa..f9fb2db7a 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -413,23 +413,29 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
int ret, i;
struct xenbus_transaction xbt;
struct xen_9pfs_front_priv *priv;
- char *versions, *v;
- unsigned int max_rings, max_ring_order, len = 0;
+ char *versions, *v, *token;
+ bool version_1 = false;
+ unsigned int max_rings, max_ring_order, len = 0, version;
versions = xenbus_read(XBT_NIL, dev->otherend, "versions", &len);
if (IS_ERR(versions))
return PTR_ERR(versions);
- for (v = versions; *v; v++) {
- if (simple_strtoul(v, &v, 10) == 1) {
- v = NULL;
- break;
+ for (v = versions; (token = strsep(&v, ",")); ) {
+ if (!*token)
+ continue;
+
+ ret = kstrtouint(token, 10, &version);
+ if (ret) {
+ kfree(versions);
+ return ret;
}
- }
- if (v) {
- kfree(versions);
- return -EINVAL;
+ if (version == 1)
+ version_1 = true;
}
kfree(versions);
+ if (!version_1)
+ return -EINVAL;
+
max_rings = xenbus_read_unsigned(dev->otherend, "max-rings", 0);
if (max_rings < XEN_9PFS_NUM_RINGS)
return -EINVAL;
--
2.47.3From: Yufan Chen <ericterminal@gmail.com>
Replace simple_strtoul() in brport_store() with kstrtoul() so
conversion failures and range errors are returned as standard errno.
This keeps parsing strict and removes deprecated helper usage.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
net/bridge/br_sysfs_if.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 1f57c36a7..cdecc7d12 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -318,7 +318,6 @@ static ssize_t brport_store(struct kobject *kobj,
struct net_bridge_port *p = kobj_to_brport(kobj);
ssize_t ret = -EINVAL;
unsigned long val;
- char *endp;
if (!ns_capable(dev_net(p->dev)->user_ns, CAP_NET_ADMIN))
return -EPERM;
@@ -339,8 +338,8 @@ static ssize_t brport_store(struct kobject *kobj,
spin_unlock_bh(&p->br->lock);
kfree(buf_copy);
} else if (brport_attr->store) {
- val = simple_strtoul(buf, &endp, 0);
- if (endp == buf)
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
goto out_unlock;
spin_lock_bh(&p->br->lock);
ret = brport_attr->store(p, val);
--
2.47.3From: Yufan Chen <ericterminal@gmail.com>
Use kstrtouint() in proc_dodebug() after trimming trailing
whitespace. This keeps accepted whitespace behavior while enforcing
full-token parsing with standard errno returns.
Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
net/sunrpc/sysctl.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index bdb587a72..07072218b 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -12,6 +12,7 @@
#include <linux/linkage.h>
#include <linux/ctype.h>
#include <linux/fs.h>
+#include <linux/kernel.h>
#include <linux/sysctl.h>
#include <linux/module.h>
@@ -65,10 +66,11 @@ static int
proc_dodebug(const struct ctl_table *table, int write, void *buffer, size_t *lenp,
loff_t *ppos)
{
- char tmpbuf[20], *s = NULL;
+ char tmpbuf[20];
char *p;
unsigned int value;
size_t left, len;
+ int ret;
if ((*ppos && !write) || !*lenp) {
*lenp = 0;
@@ -89,19 +91,17 @@ proc_dodebug(const struct ctl_table *table, int write, void *buffer, size_t *len
if (left > sizeof(tmpbuf) - 1)
return -EINVAL;
memcpy(tmpbuf, p, left);
+
+ while (left && isspace(tmpbuf[left - 1]))
+ left--;
tmpbuf[left] = '\0';
+ if (!tmpbuf[0])
+ goto done;
- value = simple_strtol(tmpbuf, &s, 0);
- if (s) {
- left -= (s - tmpbuf);
- if (left && !isspace(*s))
- return -EINVAL;
- while (left && isspace(*s)) {
- left--;
- s++;
- }
- } else
- left = 0;
+ ret = kstrtouint(tmpbuf, 0, &value);
+ if (ret)
+ return ret;
+ left = 0;
*(unsigned int *) table->data = value;
/* Display the RPC tasks on writing to rpc_debug */
if (strcmp(table->procname, "rpc_debug") == 0)
--
2.47.3© 2016 - 2026 Red Hat, Inc.