1. Patchew
  2. linux
  3. View series

[PATCH v2] vsock: ignore empty child namespace mode writes

Aldo Ariel Panzardo posted 1 patch 1 week, 2 days ago
There is a newer version of this series
net/vmw_vsock/af_vsock.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2] vsock: ignore empty child namespace mode writes
Posted by Aldo Ariel Panzardo 1 week, 2 days ago 
__vsock_net_mode_string() returns success without updating new_mode when
the transfer length is zero. Its caller then reads the uninitialized enum
and may permanently store a stack-derived value in the write-once child
mode.

Return before inspecting or storing new_mode when no bytes were
transferred. This also prevents an empty write from locking the current
mode.

Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 net/vmw_vsock/af_vsock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 622dbd0467..816d25b314 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2883,6 +2883,8 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
 				      vsock_net_child_mode(net), &new_mode);
 	if (ret)
 		return ret;
+	if (!*lenp)
+		return 0;
 
 	if (write) {
 		/* Prevent a "local" namespace from escalating to "global",
-- 
2.43.0
Re: [PATCH v2] vsock: ignore empty child namespace mode writes
Posted by Aldo Ariel Panzardo 1 week, 2 days ago 
Hi Luigi, Stefano,

Thanks for taking a look.

Luigi — nothing fancy, I was just reading sysctl handlers and
noticed that __vsock_net_mode_string() bails early on a zero-length
write without touching new_mode. Since the child mode can only be
set once, an empty write that sneaks through locks it with whatever
was on the stack. CONFIG_INIT_STACK_ALL_ZERO hides the whole thing,
which is probably why nobody hit it in practice.

Stefano — yeah, makes more sense to bail before calling the helper
at all. Fixed in v3, and I added the CCs I missed.

Cheers,
Aldo
Re: [PATCH v2] vsock: ignore empty child namespace mode writes
Posted by Stefano Garzarella 1 week, 2 days ago 
CCing Bobby (and I guess you're missing several maintainers in CC)

On Tue, Sep 15, 2026 at 11:15:47AM -0300, Aldo Ariel Panzardo wrote:
>__vsock_net_mode_string() returns success without updating new_mode when
>the transfer length is zero. Its caller then reads the uninitialized enum
>and may permanently store a stack-derived value in the write-once child
>mode.
>
>Return before inspecting or storing new_mode when no bytes were
>transferred. This also prevents an empty write from locking the current
>mode.
>
>Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
>Cc: stable@vger.kernel.org
>Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
>---
> net/vmw_vsock/af_vsock.c | 2 ++
> 1 file changed, 2 insertions(+)

A changelog after --- or a reply to v1 to explain why sending a v2, 
would be nice to have.

>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd0467..816d25b314 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2883,6 +2883,8 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
> 				      vsock_net_child_mode(net), &new_mode);
> 	if (ret)
> 		return ret;
>+	if (!*lenp)
>+		return 0;

Better to move this before calling __vsock_net_mode_string(), no?

That said, __vsock_net_mode_string() needs some cleanup IMO but I guess 
something for net-next.

Thanks,
Stefano

>
> 	if (write) {
> 		/* Prevent a "local" namespace from escalating to "global",
>-- 
>2.43.0
>
Re: [PATCH v2] vsock: ignore empty child namespace mode writes
Posted by Luigi Leonardi 1 week, 2 days ago 
Hi Aldo,

On Tue, Sep 15, 2026 at 11:15:47AM -0300, Aldo Ariel Panzardo wrote:
>__vsock_net_mode_string() returns success without updating new_mode when
>the transfer length is zero. Its caller then reads the uninitialized enum
>and may permanently store a stack-derived value in the write-once child
>mode.
>
>Return before inspecting or storing new_mode when no bytes were
>transferred. This also prevents an empty write from locking the current
>mode.
>
>Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
>Cc: stable@vger.kernel.org
>Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
>---
> net/vmw_vsock/af_vsock.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd0467..816d25b314 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2883,6 +2883,8 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
> 				      vsock_net_child_mode(net), &new_mode);
> 	if (ret)
> 		return ret;
>+	if (!*lenp)
>+		return 0;
>
> 	if (write) {
> 		/* Prevent a "local" namespace from escalating to "global",
>-- 
>2.43.0
>

Code LGTM.
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>

To reproduce it, I suppose we need a kernel that does not come with
`CONFIG_INIT_STACK_ALL_ZERO` set.

@Stefano do you think it's worth adding a test for this case?

Just out of curiosity, how did you find this bug?

Thanks,
Luigi
Re: [PATCH v2] vsock: ignore empty child namespace mode writes
Posted by Stefano Garzarella 1 week, 2 days ago 
On Tue, Sep 15, 2026 at 06:32:59PM +0200, Luigi Leonardi wrote:
>Hi Aldo,
>
>On Tue, Sep 15, 2026 at 11:15:47AM -0300, Aldo Ariel Panzardo wrote:
>>__vsock_net_mode_string() returns success without updating new_mode when
>>the transfer length is zero. Its caller then reads the uninitialized enum
>>and may permanently store a stack-derived value in the write-once child
>>mode.
>>
>>Return before inspecting or storing new_mode when no bytes were
>>transferred. This also prevents an empty write from locking the current
>>mode.
>>
>>Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
>>Cc: stable@vger.kernel.org
>>Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
>>---
>>net/vmw_vsock/af_vsock.c | 2 ++
>>1 file changed, 2 insertions(+)
>>
>>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>>index 622dbd0467..816d25b314 100644
>>--- a/net/vmw_vsock/af_vsock.c
>>+++ b/net/vmw_vsock/af_vsock.c
>>@@ -2883,6 +2883,8 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
>>				      vsock_net_child_mode(net), &new_mode);
>>	if (ret)
>>		return ret;
>>+	if (!*lenp)
>>+		return 0;
>>
>>	if (write) {
>>		/* Prevent a "local" namespace from escalating to "global",
>>-- 
>>2.43.0
>>
>
>Code LGTM.
>Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
>
>To reproduce it, I suppose we need a kernel that does not come with
>`CONFIG_INIT_STACK_ALL_ZERO` set.
>
>@Stefano do you think it's worth adding a test for this case?

Not a strong opinion on this on my side.

Thanks,
Stefano

>
>Just out of curiosity, how did you find this bug?
>
>Thanks,
>Luigi
>
[PATCH v3] vsock: ignore empty child namespace mode writes
Posted by Aldo Ariel Panzardo 1 week, 2 days ago 
__vsock_net_mode_string() returns success without updating new_mode when
the transfer length is zero. Its caller then reads the uninitialized enum
and may permanently store a stack-derived value in the write-once child
mode.

Return before calling __vsock_net_mode_string() when *lenp is zero so
that the helper is never invoked with nothing to parse and new_mode is
never read uninitialized. This also prevents an empty write from
locking the current mode.

Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
Cc: stable@vger.kernel.org
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

Changes in v3:
  - Move the !*lenp early return before __vsock_net_mode_string()
    instead of after it, as suggested by Stefano Garzarella.
  - Add missing maintainer CCs per get_maintainer.pl.
  - Add Luigi's Reviewed-by.

Changes in v2:
  - Return early instead of initializing new_mode to the current
    value, which would silently consume the write-once transition.

 net/vmw_vsock/af_vsock.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 622dbd0467..XXXXXXX 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2879,6 +2879,9 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,

 	net = container_of(table->data, struct net, vsock.child_ns_mode);

+	if (!*lenp)
+		return 0;
+
 	ret = __vsock_net_mode_string(table, write, buffer, lenp, ppos,
 				      vsock_net_child_mode(net), &new_mode);
 	if (ret)

--
2.43.0
Re: [PATCH v3] vsock: ignore empty child namespace mode writes
Posted by Bobby Eshleman 1 week, 1 day ago 
On Tue, Sep 15, 2026 at 02:30:50PM -0300, Aldo Ariel Panzardo wrote:
> __vsock_net_mode_string() returns success without updating new_mode when
> the transfer length is zero. Its caller then reads the uninitialized enum
> and may permanently store a stack-derived value in the write-once child
> mode.
> 
> Return before calling __vsock_net_mode_string() when *lenp is zero so
> that the helper is never invoked with nothing to parse and new_mode is
> never read uninitialized. This also prevents an empty write from
> locking the current mode.
> 
> Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
> Cc: stable@vger.kernel.org
> Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> 
> Changes in v3:
>   - Move the !*lenp early return before __vsock_net_mode_string()
>     instead of after it, as suggested by Stefano Garzarella.
>   - Add missing maintainer CCs per get_maintainer.pl.
>   - Add Luigi's Reviewed-by.
> 
> Changes in v2:
>   - Return early instead of initializing new_mode to the current
>     value, which would silently consume the write-once transition.
> 
>  net/vmw_vsock/af_vsock.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 622dbd0467..XXXXXXX 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -2879,6 +2879,9 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
> 
>  	net = container_of(table->data, struct net, vsock.child_ns_mode);
> 
> +	if (!*lenp)
> +		return 0;
> +
>  	ret = __vsock_net_mode_string(table, write, buffer, lenp, ppos,
>  				      vsock_net_child_mode(net), &new_mode);
>  	if (ret)
> 
> --
> 2.43.0

Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
Re: [PATCH v3] vsock: ignore empty child namespace mode writes
Posted by Stefano Garzarella 1 week, 1 day ago 
On Tue, Sep 15, 2026 at 02:30:50PM -0300, Aldo Ariel Panzardo wrote:
>__vsock_net_mode_string() returns success without updating new_mode when
>the transfer length is zero. Its caller then reads the uninitialized enum
>and may permanently store a stack-derived value in the write-once child
>mode.
>
>Return before calling __vsock_net_mode_string() when *lenp is zero so
>that the helper is never invoked with nothing to parse and new_mode is
>never read uninitialized. This also prevents an empty write from
>locking the current mode.
>
>Fixes: eafb64f40ca4 ("vsock: add netns to vsock core")
>Cc: stable@vger.kernel.org
>Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
>Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
>---
>
>Changes in v3:
>  - Move the !*lenp early return before __vsock_net_mode_string()
>    instead of after it, as suggested by Stefano Garzarella.
>  - Add missing maintainer CCs per get_maintainer.pl.
>  - Add Luigi's Reviewed-by.
>
>Changes in v2:
>  - Return early instead of initializing new_mode to the current
>    value, which would silently consume the write-once transition.
>
> net/vmw_vsock/af_vsock.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd0467..XXXXXXX 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2879,6 +2879,9 @@ static int vsock_net_child_mode_string(const struct ctl_table *table, int write,
>
> 	net = container_of(table->data, struct net, vsock.child_ns_mode);
>
>+	if (!*lenp)
>+		return 0;
>+

I would have moved it even before the net assignmet, but in any case, it 
doesn't make much difference:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.