Netdev List
 help / color / mirror / Atom feed
* [PATCH] sunrpc: fix uninitialized xprt_create_args structure
@ 2026-06-02  8:32 Hongling Zeng
  2026-06-02 10:30 ` Jeff Layton
  2026-06-02 15:59 ` Anna Schumaker
  0 siblings, 2 replies; 4+ messages in thread
From: Hongling Zeng @ 2026-06-02  8:32 UTC (permalink / raw)
  To: chuck.lever, jlayton, neil, okorniev, Dai.Ngo, tom, trondmy, anna,
	davem, edumazet, kuba, pabeni, horms
  Cc: linux-nfs, netdev, zhongling0719, Hongling Zeng, stable

The xprt_create_args structure is allocated on the stack without
initialization in rpc_sysfs_xprt_switch_add_xprt_store(). While some
fields are manually populated, critical fields like srcaddr, bc_xps,
and flags contain uninitialized stack garbage.

This can lead to:
1. Kernel panic when xs_setup_xprt() dereferences garbage srcaddr
2. Information leak if srcaddr points to sensitive stack data
3. Unpredictable behavior if flags has random bits set

The fix is to zero-initialize the structure to ensure all unused
fields are NULL/0, preventing the transport setup code from acting
on garbage data.

Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 net/sunrpc/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
index a90480f80154..0a99d0f1eb4c 100644
--- a/net/sunrpc/sysfs.c
+++ b/net/sunrpc/sysfs.c
@@ -333,6 +333,7 @@ static ssize_t rpc_sysfs_xprt_switch_add_xprt_store(struct kobject *kobj,
 	if (!xprt_switch)
 		return 0;
 
+	memset(&xprt_create_args, 0, sizeof(xprt_create_args));
 	xprt = rpc_xprt_switch_get_main_xprt(xprt_switch);
 	if (!xprt)
 		goto out;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sunrpc: fix uninitialized xprt_create_args structure
  2026-06-02  8:32 [PATCH] sunrpc: fix uninitialized xprt_create_args structure Hongling Zeng
@ 2026-06-02 10:30 ` Jeff Layton
  2026-06-02 15:59 ` Anna Schumaker
  1 sibling, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2026-06-02 10:30 UTC (permalink / raw)
  To: Hongling Zeng, chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy,
	anna, davem, edumazet, kuba, pabeni, horms
  Cc: linux-nfs, netdev, zhongling0719, stable

On Tue, 2026-06-02 at 16:32 +0800, Hongling Zeng wrote:
> The xprt_create_args structure is allocated on the stack without
> initialization in rpc_sysfs_xprt_switch_add_xprt_store(). While some
> fields are manually populated, critical fields like srcaddr, bc_xps,
> and flags contain uninitialized stack garbage.
> 
> This can lead to:
> 1. Kernel panic when xs_setup_xprt() dereferences garbage srcaddr
> 2. Information leak if srcaddr points to sensitive stack data
> 3. Unpredictable behavior if flags has random bits set
> 
> The fix is to zero-initialize the structure to ensure all unused
> fields are NULL/0, preventing the transport setup code from acting
> on garbage data.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  net/sunrpc/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index a90480f80154..0a99d0f1eb4c 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -333,6 +333,7 @@ static ssize_t rpc_sysfs_xprt_switch_add_xprt_store(struct kobject *kobj,
>  	if (!xprt_switch)
>  		return 0;
>  
> +	memset(&xprt_create_args, 0, sizeof(xprt_create_args));
>  	xprt = rpc_xprt_switch_get_main_xprt(xprt_switch);
>  	if (!xprt)
>  		goto out;

Good catch. You could also just use an initializer here at declaration.
A'la:

	struct xprt_create xprt_create_args = {};

...but this works too, of course.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sunrpc: fix uninitialized xprt_create_args structure
  2026-06-02  8:32 [PATCH] sunrpc: fix uninitialized xprt_create_args structure Hongling Zeng
  2026-06-02 10:30 ` Jeff Layton
@ 2026-06-02 15:59 ` Anna Schumaker
  2026-06-03  1:33   ` Hongling Zeng
  1 sibling, 1 reply; 4+ messages in thread
From: Anna Schumaker @ 2026-06-02 15:59 UTC (permalink / raw)
  To: Hongling Zeng, Chuck Lever, Jeff Layton, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: linux-nfs, netdev, zhongling0719, stable

Hi Hongling,

Thanks for the patch!

On Tue, Jun 2, 2026, at 4:32 AM, Hongling Zeng wrote:
> The xprt_create_args structure is allocated on the stack without
> initialization in rpc_sysfs_xprt_switch_add_xprt_store(). While some
> fields are manually populated, critical fields like srcaddr, bc_xps,
> and flags contain uninitialized stack garbage.
>
> This can lead to:
> 1. Kernel panic when xs_setup_xprt() dereferences garbage srcaddr
> 2. Information leak if srcaddr points to sensitive stack data
> 3. Unpredictable behavior if flags has random bits set

I took a look through the transport setup function to see what they
do when these fields are set to NULL, and it looks like thy do their
best to choose a default value which might be different than the
values set to the original transport that we are trying to clone.

Can we instead set the missing fields in the xprt_create_args based
on how the main xprt is configured?

Thanks,
Anna

>
> The fix is to zero-initialize the structure to ensure all unused
> fields are NULL/0, preventing the transport setup code from acting
> on garbage data.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  net/sunrpc/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
> index a90480f80154..0a99d0f1eb4c 100644
> --- a/net/sunrpc/sysfs.c
> +++ b/net/sunrpc/sysfs.c
> @@ -333,6 +333,7 @@ static ssize_t 
> rpc_sysfs_xprt_switch_add_xprt_store(struct kobject *kobj,
>  	if (!xprt_switch)
>  		return 0;
> 
> +	memset(&xprt_create_args, 0, sizeof(xprt_create_args));
>  	xprt = rpc_xprt_switch_get_main_xprt(xprt_switch);
>  	if (!xprt)
>  		goto out;
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sunrpc: fix uninitialized xprt_create_args structure
  2026-06-02 15:59 ` Anna Schumaker
@ 2026-06-03  1:33   ` Hongling Zeng
  0 siblings, 0 replies; 4+ messages in thread
From: Hongling Zeng @ 2026-06-03  1:33 UTC (permalink / raw)
  To: Anna Schumaker, Hongling Zeng, Chuck Lever, Jeff Layton,
	NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	Trond Myklebust, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: linux-nfs, netdev, stable

   Hi Anna and Jeff,

   Thanks for the review!

   Regarding Anna's question about copying fields from the main xprt:

   Based on my analysis, these missing fields are not stored in
   struct rpc_xprt:
   - srcaddr: only passed during creation, not stored in xprt
   - bc_xps: not present in rpc_xprt (only bc_xprt is stored)
   - flags: not stored in xprt after creation

   Since these values are not available in the main xprt, we cannot
   copy them. Zero-initializing is the correct approach with the
   current design.

   Updated to v2 with designated initializer as suggested by Jeff.

   Thanks,
   Hongling

在 2026年06月02日 23:59, Anna Schumaker 写道:
> Hi Hongling,
>
> Thanks for the patch!
>
> On Tue, Jun 2, 2026, at 4:32 AM, Hongling Zeng wrote:
>> The xprt_create_args structure is allocated on the stack without
>> initialization in rpc_sysfs_xprt_switch_add_xprt_store(). While some
>> fields are manually populated, critical fields like srcaddr, bc_xps,
>> and flags contain uninitialized stack garbage.
>>
>> This can lead to:
>> 1. Kernel panic when xs_setup_xprt() dereferences garbage srcaddr
>> 2. Information leak if srcaddr points to sensitive stack data
>> 3. Unpredictable behavior if flags has random bits set
> I took a look through the transport setup function to see what they
> do when these fields are set to NULL, and it looks like thy do their
> best to choose a default value which might be different than the
> values set to the original transport that we are trying to clone.
>
> Can we instead set the missing fields in the xprt_create_args based
> on how the main xprt is configured?
>
> Thanks,
> Anna
>
>> The fix is to zero-initialize the structure to ensure all unused
>> fields are NULL/0, preventing the transport setup code from acting
>> on garbage data.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   net/sunrpc/sysfs.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/net/sunrpc/sysfs.c b/net/sunrpc/sysfs.c
>> index a90480f80154..0a99d0f1eb4c 100644
>> --- a/net/sunrpc/sysfs.c
>> +++ b/net/sunrpc/sysfs.c
>> @@ -333,6 +333,7 @@ static ssize_t
>> rpc_sysfs_xprt_switch_add_xprt_store(struct kobject *kobj,
>>   	if (!xprt_switch)
>>   		return 0;
>>
>> +	memset(&xprt_create_args, 0, sizeof(xprt_create_args));
>>   	xprt = rpc_xprt_switch_get_main_xprt(xprt_switch);
>>   	if (!xprt)
>>   		goto out;
>> -- 
>> 2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-03  1:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-02  8:32 [PATCH] sunrpc: fix uninitialized xprt_create_args structure Hongling Zeng
2026-06-02 10:30 ` Jeff Layton
2026-06-02 15:59 ` Anna Schumaker
2026-06-03  1:33   ` Hongling Zeng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox