Re: [PATCH] sunrpc: fix uninitialized xprt_create_args structure
Hongling Zeng <[email protected]>
| Newsgroups | gmane.linux.kernel.stable,gmane.linux.nfs,gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
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: [email protected] >> Signed-off-by: Hongling Zeng <[email protected]> >> --- >> 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