Re: [PATCH] nfsd: make a copy of struct iattr before calling notify_change

From: Chuck Lever III
Date: Fri May 19 2023 - 09:36:07 EST




> On May 19, 2023, at 6:10 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Wed, 2023-05-17 at 12:26 -0400, Jeff Layton wrote:
>> notify_change can modify the iattr structure. In particular it can can
>> end up setting ATTR_MODE when ATTR_KILL_SUID is already set, causing a
>> BUG() if the same iattr is passed to notify_change more than once.
>>
>> Make a copy of the struct iattr before calling notify_change.
>>
>> Fixes: 34b91dda7124 NFSD: Make nfsd4_setattr() wait before returning NFS4ERR_DELAY
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2207969
>> Reported-by: Zhi Li <yieli@xxxxxxxxxx>
>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> ---
>> fs/nfsd/vfs.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>> index c4ef24c5ffd0..ad0c5cd900b1 100644
>> --- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -538,7 +538,9 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>
>> inode_lock(inode);
>> for (retries = 1;;) {
>> - host_err = __nfsd_setattr(dentry, iap);
>> + struct iattr attrs = *iap;
>> +
>> + host_err = __nfsd_setattr(dentry, &attrs);
>> if (host_err != -EAGAIN || !retries--)
>> break;
>> if (!nfsd_wait_for_delegreturn(rqstp, inode))
>
> Zhi Li tested the test kernel for this today and this seems to have
> fixed the issue. I think you can add:
>
> Tested-by: Zhi Li <yieli@xxxxxxxxxx>

Thanks to you both. Applied to nfsd-fixes for the next 6.4-rc PR.

--
Chuck Lever