This reverts commit 478ba09edc1f2f2ee27180a06150cb2d1a686f9c.
That commit was meant as a fix for setattrs with by fd (e.g. ftruncate) to use an open fid instead of the first fid it found on lookup. The proper fix for that is to use the fid associated with the open file struct, available in iattr->ia_file for such operations, and was actually done just before in 66246641609b ("9p: retrieve fid from file when file instance exist.") As such, this commit is no longer required.
Furthermore, changing lookup to return open fids first had unwanted side effects, as it turns out the protocol forbids the use of open fids for further walks (e.g. clone_fid) and we broke mounts for some servers enforcing this rule.
Note this only reverts to the old working behaviour, but it's still possible for lookup to return open fids if dentry->d_fsdata is not set, so more work is needed to make sure we respect this rule in the future, for example by adding a flag to the lookup functions to only match certain fid open modes depending on caller requirements.
Fixes: 478ba09edc1f ("fs/9p: search open fids first") Cc: stable@vger.kernel.org # v5.11+ Reported-by: ron minnich rminnich@gmail.com Reported-by: ng@0x80.stream Signed-off-by: Dominique Martinet asmadeus@codewreck.org ---
I'm sorry I didn't find time to check this properly fixes the clone open fid issues, but Ron reported it did so I'll assume it did for now. I'll try to find time to either implement the check in ganesha or use another server -- if you have a suggestion that'd run either a ramfs or export a local filesystem from linux I'm all ears, I couldn't get go9p to work in the very little time I tried.
I did however check that Greg's original open/chmod 0/ftruncate pattern works (while truncate was refused). Also, before revert the truncate by path wasn't refused, and now is again, so that's definitely good.
I've also tested open-unlink-ftruncate and it works properly with ganesha, but not with qemu -- it looks like qemu tries to access the file by path in setattr even if the fid has an associated fd, so that'd be a qemu bug, but it's unrelated to this patch anyway.
Unless there are issues with this patch I'll send it to Linus around Friday
fs/9p/fid.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/9p/fid.c b/fs/9p/fid.c index 6aab046c98e2..79df61fe0e59 100644 --- a/fs/9p/fid.c +++ b/fs/9p/fid.c @@ -96,12 +96,8 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any) dentry, dentry, from_kuid(&init_user_ns, uid), any); ret = NULL; - - if (d_inode(dentry)) - ret = v9fs_fid_find_inode(d_inode(dentry), uid); - /* we'll recheck under lock if there's anything to look in */ - if (!ret && dentry->d_fsdata) { + if (dentry->d_fsdata) { struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata;
spin_lock(&dentry->d_lock); @@ -113,6 +109,9 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any) } } spin_unlock(&dentry->d_lock); + } else { + if (dentry->d_inode) + ret = v9fs_fid_find_inode(dentry->d_inode, uid); }
return ret;