On 2023/04/20 22:41, Chuck Lever III wrote: >> That said, nfsd_listxattr() does: >> >> dentry = fhp->fh_dentry; >> inode = d_inode(dentry); >> *lenp = 0; >> >> inode_lock_shared(inode); >> >> len = vfs_listxattr(dentry, NULL, 0); >> >> Given that a dentry pointing to an inode *must* hold an active >> reference to that inode, I don't see how it is possible this code >> path could be using an unreferenced inode. >> >> nfsd_getxattr() has a similar code fragment to obtain the inode as >> well, so same goes for that... > > Dave, thanks for handling the due diligence! I was not 100% sure > about code that handles xattrs rather than the primary byte stream > of a file. > > Tetsuo, you can send a v2, or just let me know and I will make > a patch to correct the GFP flags. So, this inode_lock_shared() was there with an intention to make sure that xattr of inode inside the exported filesystem does not change between vfs_listxattr(dentry, NULL, 0) and vfs_listxattr(dentry, buf, len), wasn't it? Then, we can remove this inode_lock_shared() by adding a "goto retry;" when vfs_listxattr(dentry, buf, len) failed with out of buffer size due to a race condition, can't we? I leave replacing inode lock with retry path and removing GFP_NOFS to you. Thank you.
