* [PATCH] NFSv4: Restore nfs4_label into copied nfs_fattr for referrals
@ 2022-05-13 13:36 Benjamin Coddington
2022-05-13 15:05 ` Trond Myklebust
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Coddington @ 2022-05-13 13:36 UTC (permalink / raw)
To: trond.myklebust, anna; +Cc: linux-nfs, smayhew
..which will fix up trying to free uninitialized nfs4_label:
PID: 790 TASK: ffff88811b43c000 CPU: 0 COMMAND: "ls"
#0 [ffffc90000857920] panic at ffffffff81b9bfde
#1 [ffffc900008579c0] do_trap at ffffffff81023a9b
#2 [ffffc90000857a10] do_error_trap at ffffffff81023b78
#3 [ffffc90000857a58] exc_stack_segment at ffffffff81be1f45
#4 [ffffc90000857a80] asm_exc_stack_segment at ffffffff81c009de
#5 [ffffc90000857b08] nfs_lookup at ffffffffa0302322 [nfs]
#6 [ffffc90000857b70] __lookup_slow at ffffffff813a4a5f
#7 [ffffc90000857c60] walk_component at ffffffff813a86c4
#8 [ffffc90000857cb8] path_lookupat at ffffffff813a9553
#9 [ffffc90000857cf0] filename_lookup at ffffffff813ab86b
Fixes: 9558a007dbc3 ("NFS: Remove the label from the nfs4_lookup_res struct")
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
fs/nfs/nfs4proc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a79f66432bd3..4566280e6ff2 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4235,6 +4235,7 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
int status = -ENOMEM;
struct page *page = NULL;
struct nfs4_fs_locations *locations = NULL;
+ struct nfs4_label *label = fattr->label;
page = alloc_page(GFP_KERNEL);
if (page == NULL)
@@ -4263,6 +4264,7 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
/* replace the lookup nfs_fattr with the locations nfs_fattr */
memcpy(fattr, &locations->fattr, sizeof(struct nfs_fattr));
+ fattr->label = label;
memset(fhandle, 0, sizeof(struct nfs_fh));
out:
if (page)
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] NFSv4: Restore nfs4_label into copied nfs_fattr for referrals
2022-05-13 13:36 [PATCH] NFSv4: Restore nfs4_label into copied nfs_fattr for referrals Benjamin Coddington
@ 2022-05-13 15:05 ` Trond Myklebust
2022-05-13 18:16 ` Benjamin Coddington
0 siblings, 1 reply; 3+ messages in thread
From: Trond Myklebust @ 2022-05-13 15:05 UTC (permalink / raw)
To: anna, bcodding; +Cc: linux-nfs, smayhew
On Fri, 2022-05-13 at 09:36 -0400, Benjamin Coddington wrote:
> ..which will fix up trying to free uninitialized nfs4_label:
>
> PID: 790 TASK: ffff88811b43c000 CPU: 0 COMMAND: "ls"
> #0 [ffffc90000857920] panic at ffffffff81b9bfde
> #1 [ffffc900008579c0] do_trap at ffffffff81023a9b
> #2 [ffffc90000857a10] do_error_trap at ffffffff81023b78
> #3 [ffffc90000857a58] exc_stack_segment at ffffffff81be1f45
> #4 [ffffc90000857a80] asm_exc_stack_segment at ffffffff81c009de
> #5 [ffffc90000857b08] nfs_lookup at ffffffffa0302322 [nfs]
> #6 [ffffc90000857b70] __lookup_slow at ffffffff813a4a5f
> #7 [ffffc90000857c60] walk_component at ffffffff813a86c4
> #8 [ffffc90000857cb8] path_lookupat at ffffffff813a9553
> #9 [ffffc90000857cf0] filename_lookup at ffffffff813ab86b
>
> Fixes: 9558a007dbc3 ("NFS: Remove the label from the nfs4_lookup_res
> struct")
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
> fs/nfs/nfs4proc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index a79f66432bd3..4566280e6ff2 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4235,6 +4235,7 @@ static int nfs4_get_referral(struct rpc_clnt
> *client, struct inode *dir,
> int status = -ENOMEM;
> struct page *page = NULL;
> struct nfs4_fs_locations *locations = NULL;
> + struct nfs4_label *label = fattr->label;
>
> page = alloc_page(GFP_KERNEL);
> if (page == NULL)
> @@ -4263,6 +4264,7 @@ static int nfs4_get_referral(struct rpc_clnt
> *client, struct inode *dir,
>
> /* replace the lookup nfs_fattr with the locations nfs_fattr
> */
> memcpy(fattr, &locations->fattr, sizeof(struct nfs_fattr));
> + fattr->label = label;
> memset(fhandle, 0, sizeof(struct nfs_fh));
> out:
> if (page)
Thanks for finding this, but wouldn't it be better just to decode the
fattr in place instead of decoding it into locations->fattr and then
doing a memcpy() to get it placed correctly?
i.e. add a level of indirection in struct nfs4_fs_locations so that
nfs4_xdr_dec_fs_locations() just uses our fattr instead of its own.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] NFSv4: Restore nfs4_label into copied nfs_fattr for referrals
2022-05-13 15:05 ` Trond Myklebust
@ 2022-05-13 18:16 ` Benjamin Coddington
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Coddington @ 2022-05-13 18:16 UTC (permalink / raw)
To: Trond Myklebust; +Cc: anna, linux-nfs, smayhew
On 13 May 2022, at 11:05, Trond Myklebust wrote:
> On Fri, 2022-05-13 at 09:36 -0400, Benjamin Coddington wrote:
>> ..which will fix up trying to free uninitialized nfs4_label:
>>
>> PID: 790 TASK: ffff88811b43c000 CPU: 0 COMMAND: "ls"
>> #0 [ffffc90000857920] panic at ffffffff81b9bfde
>> #1 [ffffc900008579c0] do_trap at ffffffff81023a9b
>> #2 [ffffc90000857a10] do_error_trap at ffffffff81023b78
>> #3 [ffffc90000857a58] exc_stack_segment at ffffffff81be1f45
>> #4 [ffffc90000857a80] asm_exc_stack_segment at ffffffff81c009de
>> #5 [ffffc90000857b08] nfs_lookup at ffffffffa0302322 [nfs]
>> #6 [ffffc90000857b70] __lookup_slow at ffffffff813a4a5f
>> #7 [ffffc90000857c60] walk_component at ffffffff813a86c4
>> #8 [ffffc90000857cb8] path_lookupat at ffffffff813a9553
>> #9 [ffffc90000857cf0] filename_lookup at ffffffff813ab86b
>>
>> Fixes: 9558a007dbc3 ("NFS: Remove the label from the nfs4_lookup_res
>> struct")
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>> fs/nfs/nfs4proc.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index a79f66432bd3..4566280e6ff2 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -4235,6 +4235,7 @@ static int nfs4_get_referral(struct rpc_clnt
>> *client, struct inode *dir,
>> int status = -ENOMEM;
>> struct page *page = NULL;
>> struct nfs4_fs_locations *locations = NULL;
>> + struct nfs4_label *label = fattr->label;
>>
>> page = alloc_page(GFP_KERNEL);
>> if (page == NULL)
>> @@ -4263,6 +4264,7 @@ static int nfs4_get_referral(struct rpc_clnt
>> *client, struct inode *dir,
>>
>> /* replace the lookup nfs_fattr with the locations
>> nfs_fattr
>> */
>> memcpy(fattr, &locations->fattr, sizeof(struct
>> nfs_fattr));
>> + fattr->label = label;
>> memset(fhandle, 0, sizeof(struct nfs_fh));
>> out:
>> if (page)
>
> Thanks for finding this, but wouldn't it be better just to decode the
> fattr in place instead of decoding it into locations->fattr and then
> doing a memcpy() to get it placed correctly?
>
> i.e. add a level of indirection in struct nfs4_fs_locations so that
> nfs4_xdr_dec_fs_locations() just uses our fattr instead of its own.
Its a much bigger change since we'll end up needing to allocate fattr
everywhere fs_locations is used after changing it to a pointer, but if
that's the way you want it I'll send it along.
Ben
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-13 18:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 13:36 [PATCH] NFSv4: Restore nfs4_label into copied nfs_fattr for referrals Benjamin Coddington
2022-05-13 15:05 ` Trond Myklebust
2022-05-13 18:16 ` Benjamin Coddington
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.