9p/xen: mark 9p transport device as closing when removing it

[PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Ariadne Conill 2 days, 9 hours ago

We need to do this so that we can signal to the other end that the
device is being removed, so that it will release its claim on the
underlying memory allocation.  Otherwise releasing the grant-table
entries is deferred resulting in a kernel oops since the pages have
already been freed.

Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Fixes: 71ebd71921e45 ("xen/9pfs: connect to the backend")
Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
Signed-off-by: Alex Zenla <alex@edera.dev>
---
 net/9p/trans_xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index b9ff69c7522a..cde283c42dc6 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -312,6 +312,7 @@ static void xen_9pfs_front_remove(struct xenbus_device *dev)
 {
 	struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
 
+	xenbus_switch_state(dev, XenbusStateClosing);
 	dev_set_drvdata(&dev->dev, NULL);
 	xen_9pfs_front_free(priv);
 }
-- 
2.52.0

Re: [PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Marek Marczykowski-Górecki 1 day, 8 hours ago

On Mon, Dec 08, 2025 at 11:51:55AM -0800, Ariadne Conill wrote:
> We need to do this so that we can signal to the other end that the
> device is being removed, so that it will release its claim on the
> underlying memory allocation.  Otherwise releasing the grant-table
> entries is deferred resulting in a kernel oops since the pages have
> already been freed.
> 
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Fixes: 71ebd71921e45 ("xen/9pfs: connect to the backend")
> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> Signed-off-by: Alex Zenla <alex@edera.dev>
> ---
>  net/9p/trans_xen.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index b9ff69c7522a..cde283c42dc6 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -312,6 +312,7 @@ static void xen_9pfs_front_remove(struct xenbus_device *dev)
>  {
>  	struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
>  
> +	xenbus_switch_state(dev, XenbusStateClosing);

I think you need to wait for the backend to acknowledge it by switching
its state, before continuing. See for example
xennet_remove()->xennet_bus_close() in drivers/net/xen-netfront.c.

>  	dev_set_drvdata(&dev->dev, NULL);
>  	xen_9pfs_front_free(priv);
>  }
> -- 
> 2.52.0
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Re: [PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Demi Marie Obenour 1 day, 18 hours ago

On 12/8/25 14:51, Ariadne Conill wrote:
> We need to do this so that we can signal to the other end that the
> device is being removed, so that it will release its claim on the
> underlying memory allocation.  Otherwise releasing the grant-table
> entries is deferred resulting in a kernel oops since the pages have
> already been freed.

I don't think this is sufficient.  The backend can simply refuse
to release the grants.  The frontend needs to ensure that the pages
are not freed until the grant table entries are freed.  Right now,
the backend can cause a use-after-free in the frontend, and my
understanding of the Xen Project's security policy is that this is
a security vulnerability in the frontend code.

My instinct is that the core Xen code should take a reference on
each page before granting it to another domain, and not release that
reference until the pages are no longer granted.  This should prevent
any use-after-free problems if I understand Linux core MM correctly.

> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Fixes: 71ebd71921e45 ("xen/9pfs: connect to the backend")
> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> Signed-off-by: Alex Zenla <alex@edera.dev>
> ---
>  net/9p/trans_xen.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index b9ff69c7522a..cde283c42dc6 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -312,6 +312,7 @@ static void xen_9pfs_front_remove(struct xenbus_device *dev)
>  {
>  	struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
>  
> +	xenbus_switch_state(dev, XenbusStateClosing);
>  	dev_set_drvdata(&dev->dev, NULL);
>  	xen_9pfs_front_free(priv);
>  }

-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

Re: [PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Ariadne Conill 1 day, 12 hours ago

Hello,

> On Dec 9, 2025, at 2:43 AM, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> 
> On 12/8/25 14:51, Ariadne Conill wrote:
>> We need to do this so that we can signal to the other end that the
>> device is being removed, so that it will release its claim on the
>> underlying memory allocation.  Otherwise releasing the grant-table
>> entries is deferred resulting in a kernel oops since the pages have
>> already been freed.
> 
> I don't think this is sufficient.  The backend can simply refuse
> to release the grants.  The frontend needs to ensure that the pages
> are not freed until the grant table entries are freed.  Right now,
> the backend can cause a use-after-free in the frontend, and my
> understanding of the Xen Project's security policy is that this is
> a security vulnerability in the frontend code.
> 
> My instinct is that the core Xen code should take a reference on
> each page before granting it to another domain, and not release that
> reference until the pages are no longer granted.  This should prevent
> any use-after-free problems if I understand Linux core MM correctly.

Yes, there are other issues in the 9p transport that are likely in play here.  In our internal testing, we confirm this is not a full fix for hotplugging 9p transport devices, but no such claim of a complete fix has been made here or in the Matrix thread.

However, this is one defect that is contributing to the overall hotplugging problem and should be merged regardless: if the driver isn’t telling the other side to disconnect, the other side will never release the grants to begin with.

Ariadne

Re: [PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Demi Marie Obenour 1 day, 12 hours ago

On 12/9/25 12:09, Ariadne Conill wrote:
> Hello,
> 
>> On Dec 9, 2025, at 2:43 AM, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>
>> On 12/8/25 14:51, Ariadne Conill wrote:
>>> We need to do this so that we can signal to the other end that the
>>> device is being removed, so that it will release its claim on the
>>> underlying memory allocation.  Otherwise releasing the grant-table
>>> entries is deferred resulting in a kernel oops since the pages have
>>> already been freed.
>>
>> I don't think this is sufficient.  The backend can simply refuse
>> to release the grants.  The frontend needs to ensure that the pages
>> are not freed until the grant table entries are freed.  Right now,
>> the backend can cause a use-after-free in the frontend, and my
>> understanding of the Xen Project's security policy is that this is
>> a security vulnerability in the frontend code.
>>
>> My instinct is that the core Xen code should take a reference on
>> each page before granting it to another domain, and not release that
>> reference until the pages are no longer granted.  This should prevent
>> any use-after-free problems if I understand Linux core MM correctly.
> 
> Yes, there are other issues in the 9p transport that are likely in play here.  In our internal testing, we confirm this is not a full fix for hotplugging 9p transport devices, but no such claim of a complete fix has been made here or in the Matrix thread.
> 
> However, this is one defect that is contributing to the overall hotplugging problem and should be merged regardless: if the driver isn’t telling the other side to disconnect, the other side will never release the grants to begin with.
> 
> Ariadne

I definitely agree that this should be merged!

Is this code-path triggerable by the backend at will, or only during
teardown by the toolstack?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

Re: [PATCH] 9p/xen: mark 9p transport device as closing when removing it

Posted by Ariadne Conill 1 day, 12 hours ago

Hello,

> On Dec 9, 2025, at 9:12 AM, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> 
> On 12/9/25 12:09, Ariadne Conill wrote:
>> Hello,
>> 
>>>> On Dec 9, 2025, at 2:43 AM, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>> 
>>> On 12/8/25 14:51, Ariadne Conill wrote:
>>>> We need to do this so that we can signal to the other end that the
>>>> device is being removed, so that it will release its claim on the
>>>> underlying memory allocation.  Otherwise releasing the grant-table
>>>> entries is deferred resulting in a kernel oops since the pages have
>>>> already been freed.
>>> 
>>> I don't think this is sufficient.  The backend can simply refuse
>>> to release the grants.  The frontend needs to ensure that the pages
>>> are not freed until the grant table entries are freed.  Right now,
>>> the backend can cause a use-after-free in the frontend, and my
>>> understanding of the Xen Project's security policy is that this is
>>> a security vulnerability in the frontend code.
>>> 
>>> My instinct is that the core Xen code should take a reference on
>>> each page before granting it to another domain, and not release that
>>> reference until the pages are no longer granted.  This should prevent
>>> any use-after-free problems if I understand Linux core MM correctly.
>> 
>> Yes, there are other issues in the 9p transport that are likely in play here.  In our internal testing, we confirm this is not a full fix for hotplugging 9p transport devices, but no such claim of a complete fix has been made here or in the Matrix thread.
>> 
>> However, this is one defect that is contributing to the overall hotplugging problem and should be merged regardless: if the driver isn’t telling the other side to disconnect, the other side will never release the grants to begin with.
>> 
>> Ariadne
> 
> I definitely agree that this should be merged!
> 
> Is this code-path triggerable by the backend at will, or only during
> teardown by the toolstack?

In practice it only happens when the toolstack tears down a 9p transport.

In theory it can happen in any situation where the backend decides to tear down the transport, which may or may not actually be mediated by the toolstack.

Ariadne