fs/9p: search open fids first

A previous patch fixed the "create-unlink-getattr" idiom: if getattr is
called on an unlinked file, we try to find an open fid attached to the
corresponding inode.

We have a similar issue with file permissions and setattr:

open("./test.txt", O_RDWR|O_CREAT, 0666) = 4
chmod("./test.txt", 0)                  = 0
truncate("./test.txt", 0)               = -1 EACCES (Permission denied)
ftruncate(4, 0)                         = -1 EACCES (Permission denied)

The failure is expected with truncate() but not with ftruncate().

This happens because the lookup code does find a matching fid in the
dentry list. Unfortunately, this is not an open fid and the server
will be forced to rely on the path name, rather than on an open file
descriptor. This is the case in QEMU: the setattr operation will use
truncate() and fail because of bad write permissions.

This patch changes the logic in the lookup code, so that we consider
open fids first. It gives a chance to the server to match this open
fid to an open file descriptor and use ftruncate() instead of truncate().
This does not change the current behaviour for truncate() and other
path name based syscalls, since file permissions are checked earlier
in the VFS layer.

With this patch, we get:

open("./test.txt", O_RDWR|O_CREAT, 0666) = 4
chmod("./test.txt", 0)                  = 0
truncate("./test.txt", 0)               = -1 EACCES (Permission denied)
ftruncate(4, 0)                         = 0

Link: http://lkml.kernel.org/r/20200923141146.90046-4-jianyong.wu@arm.com
Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
This commit is contained in:
Greg Kurz 2020-09-23 22:11:45 +08:00 committed by Dominique Martinet
parent 987a648509
commit 478ba09edc

View File

@ -95,8 +95,12 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
dentry, dentry, from_kuid(&init_user_ns, uid), dentry, dentry, from_kuid(&init_user_ns, uid),
any); any);
ret = NULL; 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 */ /* we'll recheck under lock if there's anything to look in */
if (dentry->d_fsdata) { if (!ret && dentry->d_fsdata) {
struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata; struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata;
spin_lock(&dentry->d_lock); spin_lock(&dentry->d_lock);
hlist_for_each_entry(fid, h, dlist) { hlist_for_each_entry(fid, h, dlist) {
@ -106,9 +110,6 @@ static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any)
} }
} }
spin_unlock(&dentry->d_lock); spin_unlock(&dentry->d_lock);
} else {
if (dentry->d_inode)
ret = v9fs_fid_find_inode(dentry->d_inode, uid);
} }
return ret; return ret;