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: [email protected] # v5.11+
Reported-by: ron minnich <[email protected]>
Reported-by: [email protected]
Signed-off-by: Dominique Martinet <[email protected]>
---
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;
--
2.34.1
Hi Linus,
I rarely send fixes out small things before rc1, for single patches do
you have a preference between a pull request or just resending the patch
again with you added to recipients after reviews?
The following changes since commit e783362eb54cd99b2cac8b3a9aeac942e6f6ac07:
Linux 5.17-rc1 (2022-01-23 10:12:53 +0200)
are available in the Git repository at:
git://github.com/martinetd/linux tags/9p-for-5.17-rc3
for you to fetch changes up to 22e424feb6658c5d6789e45121830357809c59cb:
Revert "fs/9p: search open fids first" (2022-01-30 22:13:37 +0900)
----------------------------------------------------------------
9p-for-5.17-rc3: fix cannot walk open fid rule
the 9p 'walk' operation requires fid arguments to not originate from
an open or create call and we've missed that for a while as the
servers regularly running tests with don't enforce the check and
no active reviewer knew about the rule.
Both reporters confirmed reverting this patch fixes things for them
and looking at it further wasn't actually required...
Will take more time for follow up and enforcing the rule more
thoroughly later.
----------------------------------------------------------------
Dominique Martinet (1):
Revert "fs/9p: search open fids first"
fs/9p/fid.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
--
Dominique
The pull request you sent on Fri, 4 Feb 2022 19:52:55 +0900:
> git://github.com/martinetd/linux tags/9p-for-5.17-rc3
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/1eb7de177d4073085e3a1cebf19d5d538d171f10
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Fri, Feb 4, 2022 at 2:53 AM Dominique Martinet
<[email protected]> wrote:
>
> I rarely send fixes out small things before rc1, for single patches do
> you have a preference between a pull request or just resending the patch
> again with you added to recipients after reviews?
Generally, pull requests are what I prefer, partly for the workflow,
partly for the signed tags, and partly because then the patch also
gets that same base commit that you tested on.
That said, despite all those reasons, it's a _very_ weak preference
when we're talking a single individual patch.
So if it's easier for you to just send a change as a patch, I still
very much apply individual patches too.
Linus