[PATCH] 9p/trans_xen: make cleanup idempotent after dataring alloc errors

[PATCH] 9p/trans_xen: make cleanup idempotent after dataring alloc errors

[Eric-Terminal]

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.3

[PATCH v2 0/4] net: replace deprecated simple_strto* parsers with kstrto*

[Eric-Terminal]

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().

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

[PATCH v2 1/4] 9p/trans_xen: make cleanup idempotent after dataring alloc errors

[Eric-Terminal]

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.3

[PATCH v2 2/4] 9p/trans_xen: replace simple_strto* with kstrtouint

[Eric-Terminal]

From: 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.3

[PATCH v2 3/4] net: bridge: replace deprecated simple_strtoul with kstrtoul

[Eric-Terminal]

From: 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.3

[PATCH v2 4/4] sunrpc: sysctl: replace simple_strtol with kstrtouint

[Eric-Terminal]

From: 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

Re: [PATCH v2 0/4] net: replace deprecated simple_strto* parsers with kstrto*

[Simon Horman]

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.

Re: [PATCH v2 0/4] net: replace deprecated simple_strto* parsers with kstrto*

[Eric_Terminal]

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,

Re: [PATCH v2 0/4] net: replace deprecated simple_strto* parsers with kstrto*

[Eric_Terminal]

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,

Re: [PATCH v2 0/4] net: replace deprecated simple_strto* parsers with kstrto*

[Dominique Martinet]

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