[PATCH] 9p/xen: fix refcount leak in p9_xen_response() on wrong tag

Yifei Gao posted 1 patch 1 week, 1 day ago
Failed in applying to current master (apply log)
There is a newer version of this series
net/9p/trans_xen.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] 9p/xen: fix refcount leak in p9_xen_response() on wrong tag
Posted by Yifei Gao 1 week, 1 day ago
p9_xen_response() looks up the request for an incoming reply with
p9_tag_lookup(), which takes a reference on the returned p9_req_t. When
the tag does not resolve to a request in REQ_STATUS_SENT, the function
warns and continues the loop without dropping that reference, permanently
leaking the p9_req_t and its msize buffers. The reply header, including
the tag, is supplied by the backend, so a malicious or buggy 9P backend
can leak kernel memory on every crafted response.

Drop the reference before continuing, mirroring the equivalent path in
trans_fd.c.

Fixes: f66c72bea129 ("xen/9pfs: receive responses")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
---
 net/9p/trans_xen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index f9fb2db7a066..8eea0da8797f 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -203,6 +203,8 @@ static void p9_xen_response(struct work_struct *work)
 		req = p9_tag_lookup(priv->client, h.tag);
 		if (!req || req->status != REQ_STATUS_SENT) {
 			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
+			if (req)
+				p9_req_put(priv->client, req);
 			cons += h.size;
 			virt_mb();
 			ring->intf->in_cons = cons;
-- 
2.43.0
Re: [PATCH] 9p/xen: fix refcount leak in p9_xen_response() on wrong tag
Posted by Stefano Stabellini 1 week, 1 day ago
On Tue, 4 Aug 2026, Yifei Gao wrote:
> p9_xen_response() looks up the request for an incoming reply with
> p9_tag_lookup(), which takes a reference on the returned p9_req_t. When
> the tag does not resolve to a request in REQ_STATUS_SENT, the function
> warns and continues the loop without dropping that reference, permanently
> leaking the p9_req_t and its msize buffers. The reply header, including
> the tag, is supplied by the backend, so a malicious or buggy 9P backend
> can leak kernel memory on every crafted response.

Most backend are trusted, including this. So I would avoid "malicious".


> Drop the reference before continuing, mirroring the equivalent path in
> trans_fd.c.
 
It doesn't look like trans_fd.c behaves like this patch?


> Fixes: f66c72bea129 ("xen/9pfs: receive responses")

This should be 728356dedeff

Aside from the above, the code change looks correct

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Yifei Gao <gyf161023@gmail.com>
> ---
>  net/9p/trans_xen.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index f9fb2db7a066..8eea0da8797f 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -203,6 +203,8 @@ static void p9_xen_response(struct work_struct *work)
>  		req = p9_tag_lookup(priv->client, h.tag);
>  		if (!req || req->status != REQ_STATUS_SENT) {
>  			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
> +			if (req)
> +				p9_req_put(priv->client, req);
>  			cons += h.size;
>  			virt_mb();
>  			ring->intf->in_cons = cons;
> -- 
> 2.43.0
>
Re: [PATCH] 9p/xen: fix refcount leak in p9_xen_response() on wrong tag
Posted by Jürgen Groß 1 week, 1 day ago
On 05.08.26 02:06, Stefano Stabellini wrote:
> On Tue, 4 Aug 2026, Yifei Gao wrote:
>> p9_xen_response() looks up the request for an incoming reply with
>> p9_tag_lookup(), which takes a reference on the returned p9_req_t. When
>> the tag does not resolve to a request in REQ_STATUS_SENT, the function
>> warns and continues the loop without dropping that reference, permanently
>> leaking the p9_req_t and its msize buffers. The reply header, including
>> the tag, is supplied by the backend, so a malicious or buggy 9P backend
>> can leak kernel memory on every crafted response.
> 
> Most backend are trusted, including this. So I would avoid "malicious".

No, I think this is fine.

Especially with driver domains malicious backends are a thing. They should
only be capable to deliver wrong or no data to the frontend, but ideally
the frontend should not trust the backend.

Any work towards that goal is to be supported IMHO, and there are already
frontends listed in Xen's support statement following this rule, so any
violation of that principle in those frontends will be regarded to be a
security issue worth an XSA.


Juergen
[PATCH v2] 9p/xen: fix refcount leak in p9_xen_response() on wrong tag
Posted by Yifei Gao 6 days, 17 hours ago
p9_xen_response() looks up the request for an incoming reply with
p9_tag_lookup(), which takes a reference on the returned p9_req_t. When
the tag does not resolve to a request in REQ_STATUS_SENT, the function
warns and continues the loop without dropping that reference, permanently
leaking the p9_req_t and its msize buffers. The reply header, including
the tag, is supplied by the backend, so a malicious or buggy 9P backend
can leak kernel memory on every crafted response.

Drop the reference before continuing.

Fixes: 728356dedeff ("9p: Add refcount to p9_req_t")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
  - Fix Fixes: tag to reference the correct commit (728356dedeff).
  - Drop the inaccurate trans_fd.c comparison from the changelog.
  - Add Reviewed-by from Stefano.


 net/9p/trans_xen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index f9fb2db7a066..8eea0da8797f 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -203,6 +203,8 @@ static void p9_xen_response(struct work_struct *work)
 		req = p9_tag_lookup(priv->client, h.tag);
 		if (!req || req->status != REQ_STATUS_SENT) {
 			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
+			if (req)
+				p9_req_put(priv->client, req);
 			cons += h.size;
 			virt_mb();
 			ring->intf->in_cons = cons;
-- 
2.43.0