* [PATCH v2 02/15] ceph: fix up error handling with snapdirs
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 03/15] ceph: don't allow type or device number to change on non-I_NEW inodes Al Viro
` (13 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
From: Jeff Layton <jlayton@kernel.org>
There are several warts in the snapdir error handling. The -EOPNOTSUPP
return in __snapfh_to_dentry is currently lost, and the call to
ceph_handle_snapdir is not currently checked at all.
Fix all of this up and eliminate a BUG_ON in ceph_get_snapdir. We can
handle that case with a warning and return an error.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/ceph/dir.c | 2 ++
fs/ceph/export.c | 9 +++++----
fs/ceph/inode.c | 14 +++++++++++++-
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 83d9358854fb..f7a790ed62c4 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -677,6 +677,8 @@ int ceph_handle_snapdir(struct ceph_mds_request *req,
strcmp(dentry->d_name.name,
fsc->mount_options->snapdir_name) == 0) {
struct inode *inode = ceph_get_snapdir(parent);
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
dout("ENOENT on snapdir %p '%pd', linking to snapdir %p\n",
dentry, dentry, inode);
BUG_ON(!d_unhashed(dentry));
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index e088843a7734..f22156ee7306 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -248,9 +248,10 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
ihold(inode);
} else {
/* mds does not support lookup snapped inode */
- err = -EOPNOTSUPP;
- inode = NULL;
+ inode = ERR_PTR(-EOPNOTSUPP);
}
+ } else {
+ inode = ERR_PTR(-ESTALE);
}
ceph_mdsc_put_request(req);
@@ -261,8 +262,8 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
dout("snapfh_to_dentry %llx.%llx parent %llx hash %x err=%d",
vino.ino, vino.snap, sfh->parent_ino, sfh->hash, err);
}
- if (!inode)
- return ERR_PTR(-ESTALE);
+ if (IS_ERR(inode))
+ return ERR_CAST(inode);
/* see comments in ceph_get_parent() */
return unlinked ? d_obtain_root(inode) : d_obtain_alias(inode);
}
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 156f849f5385..5db7bf4c6a26 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -78,9 +78,21 @@ struct inode *ceph_get_snapdir(struct inode *parent)
struct inode *inode = ceph_get_inode(parent->i_sb, vino);
struct ceph_inode_info *ci = ceph_inode(inode);
- BUG_ON(!S_ISDIR(parent->i_mode));
if (IS_ERR(inode))
return inode;
+
+ if (!S_ISDIR(parent->i_mode)) {
+ pr_warn_once("bad snapdir parent type (mode=0%o)\n",
+ parent->i_mode);
+ return ERR_PTR(-ENOTDIR);
+ }
+
+ if (!(inode->i_state & I_NEW) && !S_ISDIR(inode->i_mode)) {
+ pr_warn_once("bad snapdir inode type (mode=0%o)\n",
+ inode->i_mode);
+ return ERR_PTR(-ENOTDIR);
+ }
+
inode->i_mode = parent->i_mode;
inode->i_uid = parent->i_uid;
inode->i_gid = parent->i_gid;
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 03/15] ceph: don't allow type or device number to change on non-I_NEW inodes
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
2021-03-13 4:38 ` [PATCH v2 02/15] ceph: fix up error handling with snapdirs Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 04/15] afs: Fix updating of i_mode due to 3rd party change Al Viro
` (12 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
From: Jeff Layton <jlayton@kernel.org>
Al pointed out that a malicious or broken MDS could change the type or
device number of a given inode number. It may also be possible for the
MDS to reuse an old inode number.
Ensure that we never allow fill_inode to change the type part of the
i_mode or the i_rdev unless I_NEW is set. Throw warnings if the MDS ever
changes these on us mid-stream, and return an error.
Don't set i_rdev directly, and rely on init_special_inode to do it.
Also, fix up error handling in the callers of ceph_get_inode.
In handle_cap_grant, check for and warn if the inode type changes, and
only overwrite the mode if it didn't.
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/ceph/caps.c | 8 +++++++-
fs/ceph/inode.c | 27 +++++++++++++++++++++++----
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 570731c4d019..3c03fa37cac4 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -3358,7 +3358,13 @@ static void handle_cap_grant(struct inode *inode,
if ((newcaps & CEPH_CAP_AUTH_SHARED) &&
(extra_info->issued & CEPH_CAP_AUTH_EXCL) == 0) {
- inode->i_mode = le32_to_cpu(grant->mode);
+ umode_t mode = le32_to_cpu(grant->mode);
+
+ if (inode_wrong_type(inode, mode))
+ pr_warn_once("inode type changed! (ino %llx.%llx is 0%o, mds says 0%o)\n",
+ ceph_vinop(inode), inode->i_mode, mode);
+ else
+ inode->i_mode = mode;
inode->i_uid = make_kuid(&init_user_ns, le32_to_cpu(grant->uid));
inode->i_gid = make_kgid(&init_user_ns, le32_to_cpu(grant->gid));
ci->i_btime = extra_info->btime;
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 5db7bf4c6a26..689e3ffd29d7 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -769,11 +769,32 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
bool queue_trunc = false;
bool new_version = false;
bool fill_inline = false;
+ umode_t mode = le32_to_cpu(info->mode);
+ dev_t rdev = le32_to_cpu(info->rdev);
dout("%s %p ino %llx.%llx v %llu had %llu\n", __func__,
inode, ceph_vinop(inode), le64_to_cpu(info->version),
ci->i_version);
+ /* Once I_NEW is cleared, we can't change type or dev numbers */
+ if (inode->i_state & I_NEW) {
+ inode->i_mode = mode;
+ } else {
+ if (inode_wrong_type(inode, mode)) {
+ pr_warn_once("inode type changed! (ino %llx.%llx is 0%o, mds says 0%o)\n",
+ ceph_vinop(inode), inode->i_mode, mode);
+ return -ESTALE;
+ }
+
+ if ((S_ISCHR(mode) || S_ISBLK(mode)) && inode->i_rdev != rdev) {
+ pr_warn_once("dev inode rdev changed! (ino %llx.%llx is %u:%u, mds says %u:%u)\n",
+ ceph_vinop(inode), MAJOR(inode->i_rdev),
+ MINOR(inode->i_rdev), MAJOR(rdev),
+ MINOR(rdev));
+ return -ESTALE;
+ }
+ }
+
info_caps = le32_to_cpu(info->cap.caps);
/* prealloc new cap struct */
@@ -827,8 +848,6 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
issued |= __ceph_caps_dirty(ci);
new_issued = ~issued & info_caps;
- /* update inode */
- inode->i_rdev = le32_to_cpu(info->rdev);
/* directories have fl_stripe_unit set to zero */
if (le32_to_cpu(info->layout.fl_stripe_unit))
inode->i_blkbits =
@@ -840,7 +859,7 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
if ((new_version || (new_issued & CEPH_CAP_AUTH_SHARED)) &&
(issued & CEPH_CAP_AUTH_EXCL) == 0) {
- inode->i_mode = le32_to_cpu(info->mode);
+ inode->i_mode = mode;
inode->i_uid = make_kuid(&init_user_ns, le32_to_cpu(info->uid));
inode->i_gid = make_kgid(&init_user_ns, le32_to_cpu(info->gid));
dout("%p mode 0%o uid.gid %d.%d\n", inode, inode->i_mode,
@@ -938,7 +957,7 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
case S_IFCHR:
case S_IFSOCK:
inode->i_blkbits = PAGE_SHIFT;
- init_special_inode(inode, inode->i_mode, inode->i_rdev);
+ init_special_inode(inode, inode->i_mode, rdev);
inode->i_op = &ceph_file_iops;
break;
case S_IFREG:
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 04/15] afs: Fix updating of i_mode due to 3rd party change
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
2021-03-13 4:38 ` [PATCH v2 02/15] ceph: fix up error handling with snapdirs Al Viro
2021-03-13 4:38 ` [PATCH v2 03/15] ceph: don't allow type or device number to change on non-I_NEW inodes Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 05/15] vboxsf: don't allow to change the inode type Al Viro
` (11 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
From: David Howells <dhowells@redhat.com>
Fix afs_apply_status() to mask off the irrelevant bits from status->mode
when OR'ing them into i_mode. This can happen when a 3rd party chmod
occurs.
Also fix afs_inode_init_from_status() to mask off the mode bits when
initialising i_mode.
Fixes: 260a980317da ("[AFS]: Add "directory write" support.")
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/afs/inode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 1156b2df28d3..9f83e671c785 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -103,13 +103,13 @@ static int afs_inode_init_from_status(struct afs_operation *op,
switch (status->type) {
case AFS_FTYPE_FILE:
- inode->i_mode = S_IFREG | status->mode;
+ inode->i_mode = S_IFREG | (status->mode & S_IALLUGO);
inode->i_op = &afs_file_inode_operations;
inode->i_fop = &afs_file_operations;
inode->i_mapping->a_ops = &afs_fs_aops;
break;
case AFS_FTYPE_DIR:
- inode->i_mode = S_IFDIR | status->mode;
+ inode->i_mode = S_IFDIR | (status->mode & S_IALLUGO);
inode->i_op = &afs_dir_inode_operations;
inode->i_fop = &afs_dir_file_operations;
inode->i_mapping->a_ops = &afs_dir_aops;
@@ -199,7 +199,7 @@ static void afs_apply_status(struct afs_operation *op,
if (status->mode != vnode->status.mode) {
mode = inode->i_mode;
mode &= ~S_IALLUGO;
- mode |= status->mode;
+ mode |= status->mode & S_IALLUGO;
WRITE_ONCE(inode->i_mode, mode);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 05/15] vboxsf: don't allow to change the inode type
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (2 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 04/15] afs: Fix updating of i_mode due to 3rd party change Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 06/15] orangefs_inode_is_stale(): i_mode type bits do *not* form a bitmap Al Viro
` (10 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
vboxsf_init_inode() is used both for initial setup of inode and for metadata
updates. Tell it whether we are updating a live inode or setting up a new
instance and have it refuse to change type in the former case.
[fixed the braino caught by Hans de Goede <hdegoede@redhat.com>]
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/vboxsf/dir.c | 4 ++--
fs/vboxsf/super.c | 4 ++--
fs/vboxsf/utils.c | 68 ++++++++++++++++++++++++++++++++++--------------------
fs/vboxsf/vfsmod.h | 4 ++--
4 files changed, 49 insertions(+), 31 deletions(-)
diff --git a/fs/vboxsf/dir.c b/fs/vboxsf/dir.c
index 7aee0ec63ade..eac6788fc6cf 100644
--- a/fs/vboxsf/dir.c
+++ b/fs/vboxsf/dir.c
@@ -225,7 +225,7 @@ static struct dentry *vboxsf_dir_lookup(struct inode *parent,
} else {
inode = vboxsf_new_inode(parent->i_sb);
if (!IS_ERR(inode))
- vboxsf_init_inode(sbi, inode, &fsinfo);
+ vboxsf_init_inode(sbi, inode, &fsinfo, false);
}
return d_splice_alias(inode, dentry);
@@ -245,7 +245,7 @@ static int vboxsf_dir_instantiate(struct inode *parent, struct dentry *dentry,
sf_i = VBOXSF_I(inode);
/* The host may have given us different attr then requested */
sf_i->force_restat = 1;
- vboxsf_init_inode(sbi, inode, info);
+ vboxsf_init_inode(sbi, inode, info, false);
d_instantiate(dentry, inode);
diff --git a/fs/vboxsf/super.c b/fs/vboxsf/super.c
index d7816c01a4f6..4f5e59f06284 100644
--- a/fs/vboxsf/super.c
+++ b/fs/vboxsf/super.c
@@ -207,7 +207,7 @@ static int vboxsf_fill_super(struct super_block *sb, struct fs_context *fc)
err = -ENOMEM;
goto fail_unmap;
}
- vboxsf_init_inode(sbi, iroot, &sbi->root_info);
+ vboxsf_init_inode(sbi, iroot, &sbi->root_info, false);
unlock_new_inode(iroot);
droot = d_make_root(iroot);
@@ -418,7 +418,7 @@ static int vboxsf_reconfigure(struct fs_context *fc)
/* Apply changed options to the root inode */
sbi->o = ctx->o;
- vboxsf_init_inode(sbi, iroot, &sbi->root_info);
+ vboxsf_init_inode(sbi, iroot, &sbi->root_info, true);
return 0;
}
diff --git a/fs/vboxsf/utils.c b/fs/vboxsf/utils.c
index 3b847e3fba24..aec2ebf7d25a 100644
--- a/fs/vboxsf/utils.c
+++ b/fs/vboxsf/utils.c
@@ -45,12 +45,12 @@ struct inode *vboxsf_new_inode(struct super_block *sb)
}
/* set [inode] attributes based on [info], uid/gid based on [sbi] */
-void vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
- const struct shfl_fsobjinfo *info)
+int vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
+ const struct shfl_fsobjinfo *info, bool reinit)
{
const struct shfl_fsobjattr *attr;
s64 allocated;
- int mode;
+ umode_t mode;
attr = &info->attr;
@@ -75,29 +75,44 @@ void vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
inode->i_mapping->a_ops = &vboxsf_reg_aops;
if (SHFL_IS_DIRECTORY(attr->mode)) {
- inode->i_mode = sbi->o.dmode_set ? sbi->o.dmode : mode;
- inode->i_mode &= ~sbi->o.dmask;
- inode->i_mode |= S_IFDIR;
- inode->i_op = &vboxsf_dir_iops;
- inode->i_fop = &vboxsf_dir_fops;
- /*
- * XXX: this probably should be set to the number of entries
- * in the directory plus two (. ..)
- */
- set_nlink(inode, 1);
+ if (sbi->o.dmode_set)
+ mode = sbi->o.dmode;
+ mode &= ~sbi->o.dmask;
+ mode |= S_IFDIR;
+ if (!reinit) {
+ inode->i_op = &vboxsf_dir_iops;
+ inode->i_fop = &vboxsf_dir_fops;
+ /*
+ * XXX: this probably should be set to the number of entries
+ * in the directory plus two (. ..)
+ */
+ set_nlink(inode, 1);
+ } else if (!S_ISDIR(inode->i_mode))
+ return -ESTALE;
+ inode->i_mode = mode;
} else if (SHFL_IS_SYMLINK(attr->mode)) {
- inode->i_mode = sbi->o.fmode_set ? sbi->o.fmode : mode;
- inode->i_mode &= ~sbi->o.fmask;
- inode->i_mode |= S_IFLNK;
- inode->i_op = &vboxsf_lnk_iops;
- set_nlink(inode, 1);
+ if (sbi->o.fmode_set)
+ mode = sbi->o.fmode;
+ mode &= ~sbi->o.fmask;
+ mode |= S_IFLNK;
+ if (!reinit) {
+ inode->i_op = &vboxsf_lnk_iops;
+ set_nlink(inode, 1);
+ } else if (!S_ISLNK(inode->i_mode))
+ return -ESTALE;
+ inode->i_mode = mode;
} else {
- inode->i_mode = sbi->o.fmode_set ? sbi->o.fmode : mode;
- inode->i_mode &= ~sbi->o.fmask;
- inode->i_mode |= S_IFREG;
- inode->i_op = &vboxsf_reg_iops;
- inode->i_fop = &vboxsf_reg_fops;
- set_nlink(inode, 1);
+ if (sbi->o.fmode_set)
+ mode = sbi->o.fmode;
+ mode &= ~sbi->o.fmask;
+ mode |= S_IFREG;
+ if (!reinit) {
+ inode->i_op = &vboxsf_reg_iops;
+ inode->i_fop = &vboxsf_reg_fops;
+ set_nlink(inode, 1);
+ } else if (!S_ISREG(inode->i_mode))
+ return -ESTALE;
+ inode->i_mode = mode;
}
inode->i_uid = sbi->o.uid;
@@ -116,6 +131,7 @@ void vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
info->change_time.ns_relative_to_unix_epoch);
inode->i_mtime = ns_to_timespec64(
info->modification_time.ns_relative_to_unix_epoch);
+ return 0;
}
int vboxsf_create_at_dentry(struct dentry *dentry,
@@ -199,7 +215,9 @@ int vboxsf_inode_revalidate(struct dentry *dentry)
dentry->d_time = jiffies;
sf_i->force_restat = 0;
- vboxsf_init_inode(sbi, inode, &info);
+ err = vboxsf_init_inode(sbi, inode, &info, true);
+ if (err)
+ return err;
/*
* If the file was changed on the host side we need to invalidate the
diff --git a/fs/vboxsf/vfsmod.h b/fs/vboxsf/vfsmod.h
index 760524e78c88..6a7a9cedebc6 100644
--- a/fs/vboxsf/vfsmod.h
+++ b/fs/vboxsf/vfsmod.h
@@ -82,8 +82,8 @@ extern const struct dentry_operations vboxsf_dentry_ops;
/* from utils.c */
struct inode *vboxsf_new_inode(struct super_block *sb);
-void vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
- const struct shfl_fsobjinfo *info);
+int vboxsf_init_inode(struct vboxsf_sbi *sbi, struct inode *inode,
+ const struct shfl_fsobjinfo *info, bool reinit);
int vboxsf_create_at_dentry(struct dentry *dentry,
struct shfl_createparms *params);
int vboxsf_stat(struct vboxsf_sbi *sbi, struct shfl_string *path,
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 06/15] orangefs_inode_is_stale(): i_mode type bits do *not* form a bitmap...
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (3 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 05/15] vboxsf: don't allow to change the inode type Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 07/15] ocfs2_inode_lock_update(): make sure we don't change the type bits of i_mode Al Viro
` (9 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/orangefs/orangefs-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c
index d4b7ae763186..46b7dcff18ac 100644
--- a/fs/orangefs/orangefs-utils.c
+++ b/fs/orangefs/orangefs-utils.c
@@ -221,7 +221,7 @@ static int orangefs_inode_is_stale(struct inode *inode,
* If the inode type or symlink target have changed then this
* inode is stale.
*/
- if (type == -1 || !(inode->i_mode & type)) {
+ if (type == -1 || inode_wrong_type(inode, type)) {
orangefs_make_bad_inode(inode);
return 1;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 07/15] ocfs2_inode_lock_update(): make sure we don't change the type bits of i_mode
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (4 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 06/15] orangefs_inode_is_stale(): i_mode type bits do *not* form a bitmap Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 08/15] gfs2: be careful with inode refresh Al Viro
` (8 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
... even if another node has gone insane. Fail with -ESTALE if it detects
an attempt to change the type bits.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/ocfs2/dlmglue.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 8e3a369086db..0fbe8bf7190f 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2204,7 +2204,7 @@ static void ocfs2_unpack_timespec(struct timespec64 *spec,
spec->tv_nsec = packed_time & OCFS2_NSEC_MASK;
}
-static void ocfs2_refresh_inode_from_lvb(struct inode *inode)
+static int ocfs2_refresh_inode_from_lvb(struct inode *inode)
{
struct ocfs2_inode_info *oi = OCFS2_I(inode);
struct ocfs2_lock_res *lockres = &oi->ip_inode_lockres;
@@ -2213,6 +2213,8 @@ static void ocfs2_refresh_inode_from_lvb(struct inode *inode)
mlog_meta_lvb(0, lockres);
lvb = ocfs2_dlm_lvb(&lockres->l_lksb);
+ if (inode_wrong_type(inode, be16_to_cpu(lvb->lvb_imode)))
+ return -ESTALE;
/* We're safe here without the lockres lock... */
spin_lock(&oi->ip_lock);
@@ -2240,6 +2242,7 @@ static void ocfs2_refresh_inode_from_lvb(struct inode *inode)
ocfs2_unpack_timespec(&inode->i_ctime,
be64_to_cpu(lvb->lvb_ictime_packed));
spin_unlock(&oi->ip_lock);
+ return 0;
}
static inline int ocfs2_meta_lvb_is_trustable(struct inode *inode,
@@ -2342,7 +2345,8 @@ static int ocfs2_inode_lock_update(struct inode *inode,
if (ocfs2_meta_lvb_is_trustable(inode, lockres)) {
mlog(0, "Trusting LVB on inode %llu\n",
(unsigned long long)oi->ip_blkno);
- ocfs2_refresh_inode_from_lvb(inode);
+ status = ocfs2_refresh_inode_from_lvb(inode);
+ goto bail_refresh;
} else {
/* Boo, we have to go to disk. */
/* read bh, cast, ocfs2_refresh_inode */
@@ -2352,6 +2356,10 @@ static int ocfs2_inode_lock_update(struct inode *inode,
goto bail_refresh;
}
fe = (struct ocfs2_dinode *) (*bh)->b_data;
+ if (inode_wrong_type(inode, le16_to_cpu(fe->i_mode))) {
+ status = -ESTALE;
+ goto bail_refresh;
+ }
/* This is a good chance to make sure we're not
* locking an invalid object. ocfs2_read_inode_block()
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 08/15] gfs2: be careful with inode refresh
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (5 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 07/15] ocfs2_inode_lock_update(): make sure we don't change the type bits of i_mode Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 09/15] do_cifs_create(): don't set ->i_mode of something we had not created Al Viro
` (7 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
1) gfs2_dinode_in() should *not* touch ->i_rdev on live inodes; even
"zero and immediately reread the same value from dinode" is broken -
have it overlap with ->release() of char device and you can get all
kinds of bogus behaviour.
2) mismatch on inode type on live inodes should be treated as fs
corruption rather than blindly setting ->i_mode.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/gfs2/glops.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 8e32d569c8bf..ef0b583c3417 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -394,18 +394,24 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
const struct gfs2_dinode *str = buf;
struct timespec64 atime;
u16 height, depth;
+ umode_t mode = be32_to_cpu(str->di_mode);
+ bool is_new = ip->i_inode.i_flags & I_NEW;
if (unlikely(ip->i_no_addr != be64_to_cpu(str->di_num.no_addr)))
goto corrupt;
+ if (unlikely(!is_new && inode_wrong_type(&ip->i_inode, mode)))
+ goto corrupt;
ip->i_no_formal_ino = be64_to_cpu(str->di_num.no_formal_ino);
- ip->i_inode.i_mode = be32_to_cpu(str->di_mode);
- ip->i_inode.i_rdev = 0;
- switch (ip->i_inode.i_mode & S_IFMT) {
- case S_IFBLK:
- case S_IFCHR:
- ip->i_inode.i_rdev = MKDEV(be32_to_cpu(str->di_major),
- be32_to_cpu(str->di_minor));
- break;
+ ip->i_inode.i_mode = mode;
+ if (is_new) {
+ ip->i_inode.i_rdev = 0;
+ switch (mode & S_IFMT) {
+ case S_IFBLK:
+ case S_IFCHR:
+ ip->i_inode.i_rdev = MKDEV(be32_to_cpu(str->di_major),
+ be32_to_cpu(str->di_minor));
+ break;
+ }
}
i_uid_write(&ip->i_inode, be32_to_cpu(str->di_uid));
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 09/15] do_cifs_create(): don't set ->i_mode of something we had not created
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (6 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 08/15] gfs2: be careful with inode refresh Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-15 17:12 ` Jeff Layton
2021-03-13 4:38 ` [PATCH v2 10/15] cifs: have ->mkdir() handle race with another client sanely Al Viro
` (6 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
If the file had existed before we'd called ->atomic_open() (without
O_EXCL, that is), we have no more business setting ->i_mode than
we would setting ->i_uid or ->i_gid. We also have no business
doing either if another client has managed to get unlink+mkdir
between ->open() and cifs_inode_get_info().
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/cifs/dir.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index a3fb81e0ba17..9d7ae93c8af7 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -418,15 +418,16 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
if (newinode) {
if (server->ops->set_lease_key)
server->ops->set_lease_key(newinode, fid);
- if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
- newinode->i_mode = mode;
- if ((*oplock & CIFS_CREATE_ACTION) &&
- (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID)) {
- newinode->i_uid = current_fsuid();
- if (inode->i_mode & S_ISGID)
- newinode->i_gid = inode->i_gid;
- else
- newinode->i_gid = current_fsgid();
+ if ((*oplock & CIFS_CREATE_ACTION) && S_ISREG(newinode->i_mode)) {
+ if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
+ newinode->i_mode = mode;
+ if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
+ newinode->i_uid = current_fsuid();
+ if (inode->i_mode & S_ISGID)
+ newinode->i_gid = inode->i_gid;
+ else
+ newinode->i_gid = current_fsgid();
+ }
}
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 09/15] do_cifs_create(): don't set ->i_mode of something we had not created
2021-03-13 4:38 ` [PATCH v2 09/15] do_cifs_create(): don't set ->i_mode of something we had not created Al Viro
@ 2021-03-15 17:12 ` Jeff Layton
0 siblings, 0 replies; 21+ messages in thread
From: Jeff Layton @ 2021-03-15 17:12 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: linux-fsdevel, David Howells, Hans de Goede, Mike Marshall,
Joseph Qi, Bob Peterson, Steve French, Richard Weinberger,
Dominique Martinet, Arnd Bergmann
On Sat, 2021-03-13 at 04:38 +0000, Al Viro wrote:
> If the file had existed before we'd called ->atomic_open() (without
> O_EXCL, that is), we have no more business setting ->i_mode than
> we would setting ->i_uid or ->i_gid. We also have no business
> doing either if another client has managed to get unlink+mkdir
> between ->open() and cifs_inode_get_info().
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/cifs/dir.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index a3fb81e0ba17..9d7ae93c8af7 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -418,15 +418,16 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
> if (newinode) {
> if (server->ops->set_lease_key)
> server->ops->set_lease_key(newinode, fid);
> - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
> - newinode->i_mode = mode;
> - if ((*oplock & CIFS_CREATE_ACTION) &&
> - (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID)) {
> - newinode->i_uid = current_fsuid();
> - if (inode->i_mode & S_ISGID)
> - newinode->i_gid = inode->i_gid;
> - else
> - newinode->i_gid = current_fsgid();
> + if ((*oplock & CIFS_CREATE_ACTION) && S_ISREG(newinode->i_mode)) {
> + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
> + newinode->i_mode = mode;
> + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
> + newinode->i_uid = current_fsuid();
> + if (inode->i_mode & S_ISGID)
> + newinode->i_gid = inode->i_gid;
> + else
> + newinode->i_gid = current_fsgid();
> + }
> }
> }
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 10/15] cifs: have ->mkdir() handle race with another client sanely
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (7 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 09/15] do_cifs_create(): don't set ->i_mode of something we had not created Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-15 17:08 ` Jeff Layton
2021-03-13 4:38 ` [PATCH v2 11/15] cifs: have cifs_fattr_to_inode() refuse to change type on live inode Al Viro
` (5 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
if we have mkdir request reported successful *and* simulating lookup
gets us a non-directory (which is possible if another client has
managed to get rmdir and create in between), the sane action is not
to mangle ->i_mode of non-directory inode to S_IFDIR | mode, it's
"report success and return with dentry negative unhashed" - that
way the next lookup will do the right thing.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/cifs/inode.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index d46b36d52211..80c487fcf10e 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1739,6 +1739,16 @@ cifs_mkdir_qinfo(struct inode *parent, struct dentry *dentry, umode_t mode,
if (rc)
return rc;
+ if (!S_ISDIR(inode->i_mode)) {
+ /*
+ * mkdir succeeded, but another client has managed to remove the
+ * sucker and replace it with non-directory. Return success,
+ * but don't leave the child in dcache.
+ */
+ iput(inode);
+ d_drop(dentry);
+ return 0;
+ }
/*
* setting nlink not necessary except in cases where we failed to get it
* from the server or was set bogus. Also, since this is a brand new
@@ -1790,7 +1800,7 @@ cifs_mkdir_qinfo(struct inode *parent, struct dentry *dentry, umode_t mode,
}
}
d_instantiate(dentry, inode);
- return rc;
+ return 0;
}
static int
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 10/15] cifs: have ->mkdir() handle race with another client sanely
2021-03-13 4:38 ` [PATCH v2 10/15] cifs: have ->mkdir() handle race with another client sanely Al Viro
@ 2021-03-15 17:08 ` Jeff Layton
0 siblings, 0 replies; 21+ messages in thread
From: Jeff Layton @ 2021-03-15 17:08 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: linux-fsdevel, David Howells, Hans de Goede, Mike Marshall,
Joseph Qi, Bob Peterson, Steve French, Richard Weinberger,
Dominique Martinet, Arnd Bergmann
On Sat, 2021-03-13 at 04:38 +0000, Al Viro wrote:
> if we have mkdir request reported successful *and* simulating lookup
> gets us a non-directory (which is possible if another client has
> managed to get rmdir and create in between), the sane action is not
> to mangle ->i_mode of non-directory inode to S_IFDIR | mode, it's
> "report success and return with dentry negative unhashed" - that
> way the next lookup will do the right thing.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/cifs/inode.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index d46b36d52211..80c487fcf10e 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1739,6 +1739,16 @@ cifs_mkdir_qinfo(struct inode *parent, struct dentry *dentry, umode_t mode,
> if (rc)
> return rc;
>
>
> + if (!S_ISDIR(inode->i_mode)) {
> + /*
> + * mkdir succeeded, but another client has managed to remove the
> + * sucker and replace it with non-directory. Return success,
> + * but don't leave the child in dcache.
> + */
> + iput(inode);
> + d_drop(dentry);
> + return 0;
> + }
> /*
> * setting nlink not necessary except in cases where we failed to get it
> * from the server or was set bogus. Also, since this is a brand new
> @@ -1790,7 +1800,7 @@ cifs_mkdir_qinfo(struct inode *parent, struct dentry *dentry, umode_t mode,
> }
> }
> d_instantiate(dentry, inode);
> - return rc;
> + return 0;
> }
>
>
> static int
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 11/15] cifs: have cifs_fattr_to_inode() refuse to change type on live inode
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (8 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 10/15] cifs: have ->mkdir() handle race with another client sanely Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-15 17:13 ` Jeff Layton
2021-03-13 4:38 ` [PATCH v2 12/15] hostfs_mknod(): don't bother with init_special_inode() Al Viro
` (4 subsequent siblings)
14 siblings, 1 reply; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
... instead of trying to do that in the callers (and missing some,
at that)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/cifs/cifsproto.h | 2 +-
fs/cifs/file.c | 2 +-
fs/cifs/inode.c | 42 +++++++++++++++---------------------------
fs/cifs/readdir.c | 4 +---
4 files changed, 18 insertions(+), 32 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 75ce6f742b8d..2a72dc24b00a 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -194,7 +194,7 @@ extern void cifs_unix_basic_to_fattr(struct cifs_fattr *fattr,
struct cifs_sb_info *cifs_sb);
extern void cifs_dir_info_to_fattr(struct cifs_fattr *, FILE_DIRECTORY_INFO *,
struct cifs_sb_info *);
-extern void cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
+extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
extern struct inode *cifs_iget(struct super_block *sb,
struct cifs_fattr *fattr);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 26de4329d161..78266f0e0595 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -165,7 +165,7 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
goto posix_open_ret;
}
} else {
- cifs_fattr_to_inode(*pinode, &fattr);
+ rc = cifs_fattr_to_inode(*pinode, &fattr);
}
posix_open_ret:
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 80c487fcf10e..51cb1ca829ec 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -157,12 +157,18 @@ cifs_nlink_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
}
/* populate an inode with info from a cifs_fattr struct */
-void
+int
cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
{
struct cifsInodeInfo *cifs_i = CIFS_I(inode);
struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+ if (!(inode->i_state & I_NEW) &&
+ unlikely(inode_wrong_type(inode, fattr->cf_mode))) {
+ CIFS_I(inode)->time = 0; /* force reval */
+ return -ESTALE;
+ }
+
cifs_revalidate_cache(inode, fattr);
spin_lock(&inode->i_lock);
@@ -219,6 +225,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
inode->i_flags |= S_AUTOMOUNT;
if (inode->i_state & I_NEW)
cifs_set_ops(inode);
+ return 0;
}
void
@@ -363,7 +370,7 @@ cifs_get_file_info_unix(struct file *filp)
rc = 0;
}
- cifs_fattr_to_inode(inode, &fattr);
+ rc = cifs_fattr_to_inode(inode, &fattr);
free_xid(xid);
return rc;
}
@@ -426,13 +433,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
}
/* if filetype is different, return error */
- if (unlikely(inode_wrong_type(*pinode, fattr.cf_mode))) {
- CIFS_I(*pinode)->time = 0; /* force reval */
- rc = -ESTALE;
- goto cgiiu_exit;
- }
-
- cifs_fattr_to_inode(*pinode, &fattr);
+ rc = cifs_fattr_to_inode(*pinode, &fattr);
}
cgiiu_exit:
@@ -782,7 +783,8 @@ cifs_get_file_info(struct file *filp)
*/
fattr.cf_uniqueid = CIFS_I(inode)->uniqueid;
fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
- cifs_fattr_to_inode(inode, &fattr);
+ /* if filetype is different, return error */
+ rc = cifs_fattr_to_inode(inode, &fattr);
cgfi_exit:
free_xid(xid);
return rc;
@@ -1099,16 +1101,8 @@ cifs_get_inode_info(struct inode **inode,
rc = -ESTALE;
goto out;
}
-
/* if filetype is different, return error */
- if (unlikely(((*inode)->i_mode & S_IFMT) !=
- (fattr.cf_mode & S_IFMT))) {
- CIFS_I(*inode)->time = 0; /* force reval */
- rc = -ESTALE;
- goto out;
- }
-
- cifs_fattr_to_inode(*inode, &fattr);
+ rc = cifs_fattr_to_inode(*inode, &fattr);
}
out:
cifs_buf_release(smb1_backup_rsp_buf);
@@ -1214,14 +1208,7 @@ smb311_posix_get_inode_info(struct inode **inode,
}
/* if filetype is different, return error */
- if (unlikely(((*inode)->i_mode & S_IFMT) !=
- (fattr.cf_mode & S_IFMT))) {
- CIFS_I(*inode)->time = 0; /* force reval */
- rc = -ESTALE;
- goto out;
- }
-
- cifs_fattr_to_inode(*inode, &fattr);
+ rc = cifs_fattr_to_inode(*inode, &fattr);
}
out:
cifs_put_tlink(tlink);
@@ -1316,6 +1303,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
}
}
+ /* can't fail - see cifs_find_inode() */
cifs_fattr_to_inode(inode, fattr);
if (sb->s_flags & SB_NOATIME)
inode->i_flags |= S_NOATIME | S_NOCMTIME;
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 80bf4c6f4c7b..e563c0fb47cb 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -119,9 +119,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
/* update inode in place
* if both i_ino and i_mode didn't change */
if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid &&
- (inode->i_mode & S_IFMT) ==
- (fattr->cf_mode & S_IFMT)) {
- cifs_fattr_to_inode(inode, fattr);
+ cifs_fattr_to_inode(inode, fattr) == 0) {
dput(dentry);
return;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 11/15] cifs: have cifs_fattr_to_inode() refuse to change type on live inode
2021-03-13 4:38 ` [PATCH v2 11/15] cifs: have cifs_fattr_to_inode() refuse to change type on live inode Al Viro
@ 2021-03-15 17:13 ` Jeff Layton
0 siblings, 0 replies; 21+ messages in thread
From: Jeff Layton @ 2021-03-15 17:13 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: linux-fsdevel, David Howells, Hans de Goede, Mike Marshall,
Joseph Qi, Bob Peterson, Steve French, Richard Weinberger,
Dominique Martinet, Arnd Bergmann
On Sat, 2021-03-13 at 04:38 +0000, Al Viro wrote:
> ... instead of trying to do that in the callers (and missing some,
> at that)
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/cifs/cifsproto.h | 2 +-
> fs/cifs/file.c | 2 +-
> fs/cifs/inode.c | 42 +++++++++++++++---------------------------
> fs/cifs/readdir.c | 4 +---
> 4 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 75ce6f742b8d..2a72dc24b00a 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -194,7 +194,7 @@ extern void cifs_unix_basic_to_fattr(struct cifs_fattr *fattr,
> struct cifs_sb_info *cifs_sb);
> extern void cifs_dir_info_to_fattr(struct cifs_fattr *, FILE_DIRECTORY_INFO *,
> struct cifs_sb_info *);
> -extern void cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
> +extern int cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr);
> extern struct inode *cifs_iget(struct super_block *sb,
> struct cifs_fattr *fattr);
>
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 26de4329d161..78266f0e0595 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -165,7 +165,7 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
> goto posix_open_ret;
> }
> } else {
> - cifs_fattr_to_inode(*pinode, &fattr);
> + rc = cifs_fattr_to_inode(*pinode, &fattr);
> }
>
>
> posix_open_ret:
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 80c487fcf10e..51cb1ca829ec 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -157,12 +157,18 @@ cifs_nlink_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
> }
>
>
> /* populate an inode with info from a cifs_fattr struct */
> -void
> +int
> cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
> {
> struct cifsInodeInfo *cifs_i = CIFS_I(inode);
> struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>
>
> + if (!(inode->i_state & I_NEW) &&
> + unlikely(inode_wrong_type(inode, fattr->cf_mode))) {
> + CIFS_I(inode)->time = 0; /* force reval */
> + return -ESTALE;
> + }
> +
> cifs_revalidate_cache(inode, fattr);
>
>
> spin_lock(&inode->i_lock);
> @@ -219,6 +225,7 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
> inode->i_flags |= S_AUTOMOUNT;
> if (inode->i_state & I_NEW)
> cifs_set_ops(inode);
> + return 0;
> }
>
>
> void
> @@ -363,7 +370,7 @@ cifs_get_file_info_unix(struct file *filp)
> rc = 0;
> }
>
>
> - cifs_fattr_to_inode(inode, &fattr);
> + rc = cifs_fattr_to_inode(inode, &fattr);
> free_xid(xid);
> return rc;
> }
> @@ -426,13 +433,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
> }
>
>
> /* if filetype is different, return error */
> - if (unlikely(inode_wrong_type(*pinode, fattr.cf_mode))) {
> - CIFS_I(*pinode)->time = 0; /* force reval */
> - rc = -ESTALE;
> - goto cgiiu_exit;
> - }
> -
> - cifs_fattr_to_inode(*pinode, &fattr);
> + rc = cifs_fattr_to_inode(*pinode, &fattr);
> }
>
>
> cgiiu_exit:
> @@ -782,7 +783,8 @@ cifs_get_file_info(struct file *filp)
> */
> fattr.cf_uniqueid = CIFS_I(inode)->uniqueid;
> fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
> - cifs_fattr_to_inode(inode, &fattr);
> + /* if filetype is different, return error */
> + rc = cifs_fattr_to_inode(inode, &fattr);
> cgfi_exit:
> free_xid(xid);
> return rc;
> @@ -1099,16 +1101,8 @@ cifs_get_inode_info(struct inode **inode,
> rc = -ESTALE;
> goto out;
> }
> -
> /* if filetype is different, return error */
> - if (unlikely(((*inode)->i_mode & S_IFMT) !=
> - (fattr.cf_mode & S_IFMT))) {
> - CIFS_I(*inode)->time = 0; /* force reval */
> - rc = -ESTALE;
> - goto out;
> - }
> -
> - cifs_fattr_to_inode(*inode, &fattr);
> + rc = cifs_fattr_to_inode(*inode, &fattr);
> }
> out:
> cifs_buf_release(smb1_backup_rsp_buf);
> @@ -1214,14 +1208,7 @@ smb311_posix_get_inode_info(struct inode **inode,
> }
>
>
> /* if filetype is different, return error */
> - if (unlikely(((*inode)->i_mode & S_IFMT) !=
> - (fattr.cf_mode & S_IFMT))) {
> - CIFS_I(*inode)->time = 0; /* force reval */
> - rc = -ESTALE;
> - goto out;
> - }
> -
> - cifs_fattr_to_inode(*inode, &fattr);
> + rc = cifs_fattr_to_inode(*inode, &fattr);
> }
> out:
> cifs_put_tlink(tlink);
> @@ -1316,6 +1303,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
> }
> }
>
>
> + /* can't fail - see cifs_find_inode() */
> cifs_fattr_to_inode(inode, fattr);
> if (sb->s_flags & SB_NOATIME)
> inode->i_flags |= S_NOATIME | S_NOCMTIME;
> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> index 80bf4c6f4c7b..e563c0fb47cb 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -119,9 +119,7 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name,
> /* update inode in place
> * if both i_ino and i_mode didn't change */
> if (CIFS_I(inode)->uniqueid == fattr->cf_uniqueid &&
> - (inode->i_mode & S_IFMT) ==
> - (fattr->cf_mode & S_IFMT)) {
> - cifs_fattr_to_inode(inode, fattr);
> + cifs_fattr_to_inode(inode, fattr) == 0) {
> dput(dentry);
> return;
> }
Nice cleanup, too.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 12/15] hostfs_mknod(): don't bother with init_special_inode()
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (9 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 11/15] cifs: have cifs_fattr_to_inode() refuse to change type on live inode Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 13/15] openpromfs: don't do unlock_new_inode() until the new inode is set up Al Viro
` (3 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
read_name() in the end of hostfs_mknod() will DTRT
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/hostfs/hostfs_kern.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index 29e407762626..aed8c4f28ad3 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -712,7 +712,6 @@ static int hostfs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
if (name == NULL)
goto out_put;
- init_special_inode(inode, mode, dev);
err = do_mknod(name, mode, MAJOR(dev), MINOR(dev));
if (err)
goto out_free;
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 13/15] openpromfs: don't do unlock_new_inode() until the new inode is set up
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (10 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 12/15] hostfs_mknod(): don't bother with init_special_inode() Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-13 4:38 ` [PATCH v2 14/15] 9p: missing chunk of "fs/9p: Don't update file type when updating file attributes" Al Viro
` (2 subsequent siblings)
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/openpromfs/inode.c | 67 +++++++++++++++++++++++++--------------------------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/fs/openpromfs/inode.c b/fs/openpromfs/inode.c
index 40c8c2e32fa3..f825176ff4ed 100644
--- a/fs/openpromfs/inode.c
+++ b/fs/openpromfs/inode.c
@@ -236,27 +236,31 @@ static struct dentry *openpromfs_lookup(struct inode *dir, struct dentry *dentry
mutex_unlock(&op_mutex);
if (IS_ERR(inode))
return ERR_CAST(inode);
- ent_oi = OP_I(inode);
- ent_oi->type = ent_type;
- ent_oi->u = ent_data;
-
- switch (ent_type) {
- case op_inode_node:
- inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
- inode->i_op = &openprom_inode_operations;
- inode->i_fop = &openprom_operations;
- set_nlink(inode, 2);
- break;
- case op_inode_prop:
- if (of_node_name_eq(dp, "options") && (len == 17) &&
- !strncmp (name, "security-password", 17))
- inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
- else
- inode->i_mode = S_IFREG | S_IRUGO;
- inode->i_fop = &openpromfs_prop_ops;
- set_nlink(inode, 1);
- inode->i_size = ent_oi->u.prop->length;
- break;
+ if (inode->i_state & I_NEW) {
+ inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+ ent_oi = OP_I(inode);
+ ent_oi->type = ent_type;
+ ent_oi->u = ent_data;
+
+ switch (ent_type) {
+ case op_inode_node:
+ inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
+ inode->i_op = &openprom_inode_operations;
+ inode->i_fop = &openprom_operations;
+ set_nlink(inode, 2);
+ break;
+ case op_inode_prop:
+ if (of_node_name_eq(dp, "options") && (len == 17) &&
+ !strncmp (name, "security-password", 17))
+ inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
+ else
+ inode->i_mode = S_IFREG | S_IRUGO;
+ inode->i_fop = &openpromfs_prop_ops;
+ set_nlink(inode, 1);
+ inode->i_size = ent_oi->u.prop->length;
+ break;
+ }
+ unlock_new_inode(inode);
}
return d_splice_alias(inode, dentry);
@@ -345,20 +349,9 @@ static void openprom_free_inode(struct inode *inode)
static struct inode *openprom_iget(struct super_block *sb, ino_t ino)
{
- struct inode *inode;
-
- inode = iget_locked(sb, ino);
+ struct inode *inode = iget_locked(sb, ino);
if (!inode)
- return ERR_PTR(-ENOMEM);
- if (inode->i_state & I_NEW) {
- inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
- if (inode->i_ino == OPENPROM_ROOT_INO) {
- inode->i_op = &openprom_inode_operations;
- inode->i_fop = &openprom_operations;
- inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
- }
- unlock_new_inode(inode);
- }
+ inode = ERR_PTR(-ENOMEM);
return inode;
}
@@ -394,9 +387,15 @@ static int openprom_fill_super(struct super_block *s, struct fs_context *fc)
goto out_no_root;
}
+ root_inode->i_mtime = root_inode->i_atime =
+ root_inode->i_ctime = current_time(root_inode);
+ root_inode->i_op = &openprom_inode_operations;
+ root_inode->i_fop = &openprom_operations;
+ root_inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
oi = OP_I(root_inode);
oi->type = op_inode_node;
oi->u.node = of_find_node_by_path("/");
+ unlock_new_inode(root_inode);
s->s_root = d_make_root(root_inode);
if (!s->s_root)
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 14/15] 9p: missing chunk of "fs/9p: Don't update file type when updating file attributes"
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (11 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 13/15] openpromfs: don't do unlock_new_inode() until the new inode is set up Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-15 17:14 ` Jeff Layton
2021-03-13 4:38 ` [PATCH v2 15/15] spufs: fix bogosity in S_ISGID handling Al Viro
2021-03-15 17:19 ` [PATCH v2 01/15] new helper: inode_wrong_type() Jeff Layton
14 siblings, 1 reply; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
In commit 45089142b149 Aneesh had missed one (admittedly, very unlikely
to hit) case in v9fs_stat2inode_dotl(). However, the same considerations
apply there as well - we have no business whatsoever to change ->i_rdev
or the file type.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/9p/vfs_inode_dotl.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index df0b87b05c42..e1c0240b51c0 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -663,14 +663,10 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
if (stat->st_result_mask & P9_STATS_NLINK)
set_nlink(inode, stat->st_nlink);
if (stat->st_result_mask & P9_STATS_MODE) {
- inode->i_mode = stat->st_mode;
- if ((S_ISBLK(inode->i_mode)) ||
- (S_ISCHR(inode->i_mode)))
- init_special_inode(inode, inode->i_mode,
- inode->i_rdev);
+ mode = stat->st_mode & S_IALLUGO;
+ mode |= inode->i_mode & ~S_IALLUGO;
+ inode->i_mode = mode;
}
- if (stat->st_result_mask & P9_STATS_RDEV)
- inode->i_rdev = new_decode_dev(stat->st_rdev);
if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE) &&
stat->st_result_mask & P9_STATS_SIZE)
v9fs_i_size_write(inode, stat->st_size);
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 14/15] 9p: missing chunk of "fs/9p: Don't update file type when updating file attributes"
2021-03-13 4:38 ` [PATCH v2 14/15] 9p: missing chunk of "fs/9p: Don't update file type when updating file attributes" Al Viro
@ 2021-03-15 17:14 ` Jeff Layton
0 siblings, 0 replies; 21+ messages in thread
From: Jeff Layton @ 2021-03-15 17:14 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: linux-fsdevel, David Howells, Hans de Goede, Mike Marshall,
Joseph Qi, Bob Peterson, Steve French, Richard Weinberger,
Dominique Martinet, Arnd Bergmann
On Sat, 2021-03-13 at 04:38 +0000, Al Viro wrote:
> In commit 45089142b149 Aneesh had missed one (admittedly, very unlikely
> to hit) case in v9fs_stat2inode_dotl(). However, the same considerations
> apply there as well - we have no business whatsoever to change ->i_rdev
> or the file type.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/9p/vfs_inode_dotl.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index df0b87b05c42..e1c0240b51c0 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -663,14 +663,10 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
> if (stat->st_result_mask & P9_STATS_NLINK)
> set_nlink(inode, stat->st_nlink);
> if (stat->st_result_mask & P9_STATS_MODE) {
> - inode->i_mode = stat->st_mode;
> - if ((S_ISBLK(inode->i_mode)) ||
> - (S_ISCHR(inode->i_mode)))
> - init_special_inode(inode, inode->i_mode,
> - inode->i_rdev);
> + mode = stat->st_mode & S_IALLUGO;
> + mode |= inode->i_mode & ~S_IALLUGO;
> + inode->i_mode = mode;
> }
> - if (stat->st_result_mask & P9_STATS_RDEV)
> - inode->i_rdev = new_decode_dev(stat->st_rdev);
> if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE) &&
> stat->st_result_mask & P9_STATS_SIZE)
> v9fs_i_size_write(inode, stat->st_size);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 15/15] spufs: fix bogosity in S_ISGID handling
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (12 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 14/15] 9p: missing chunk of "fs/9p: Don't update file type when updating file attributes" Al Viro
@ 2021-03-13 4:38 ` Al Viro
2021-03-15 17:19 ` [PATCH v2 01/15] new helper: inode_wrong_type() Jeff Layton
14 siblings, 0 replies; 21+ messages in thread
From: Al Viro @ 2021-03-13 4:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-fsdevel, Jeff Layton, David Howells, Hans de Goede,
Mike Marshall, Joseph Qi, Bob Peterson, Steve French,
Richard Weinberger, Dominique Martinet, Arnd Bergmann
clearing everything *except* S_ISGID (including the S_IFDIR, among
other things) is wrong. Just use init_inode_owner() and be done
with that...
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/powerpc/platforms/cell/spufs/inode.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index b83a3670bd74..bed05b644c2c 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -236,10 +236,7 @@ spufs_mkdir(struct inode *dir, struct dentry *dentry, unsigned int flags,
if (!inode)
return -ENOSPC;
- if (dir->i_mode & S_ISGID) {
- inode->i_gid = dir->i_gid;
- inode->i_mode &= S_ISGID;
- }
+ inode_init_owner(&init_user_ns, inode, dir, mode | S_IFDIR);
ctx = alloc_spu_context(SPUFS_I(dir)->i_gang); /* XXX gang */
SPUFS_I(inode)->i_ctx = ctx;
if (!ctx) {
@@ -470,10 +467,7 @@ spufs_mkgang(struct inode *dir, struct dentry *dentry, umode_t mode)
goto out;
ret = 0;
- if (dir->i_mode & S_ISGID) {
- inode->i_gid = dir->i_gid;
- inode->i_mode &= S_ISGID;
- }
+ inode_init_owner(&init_user_ns, inode, dir, mode | S_IFDIR);
gang = alloc_spu_gang();
SPUFS_I(inode)->i_ctx = NULL;
SPUFS_I(inode)->i_gang = gang;
--
2.11.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 01/15] new helper: inode_wrong_type()
2021-03-13 4:38 ` [PATCH v2 01/15] new helper: inode_wrong_type() Al Viro
` (13 preceding siblings ...)
2021-03-13 4:38 ` [PATCH v2 15/15] spufs: fix bogosity in S_ISGID handling Al Viro
@ 2021-03-15 17:19 ` Jeff Layton
14 siblings, 0 replies; 21+ messages in thread
From: Jeff Layton @ 2021-03-15 17:19 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: linux-fsdevel, David Howells, Hans de Goede, Mike Marshall,
Joseph Qi, Bob Peterson, Steve French, Richard Weinberger,
Dominique Martinet, Arnd Bergmann
On Sat, 2021-03-13 at 04:38 +0000, Al Viro wrote:
> inode_wrong_type(inode, mode) returns true if setting inode->i_mode
> to given value would've changed the inode type. We have enough of
> those checks open-coded to make a helper worthwhile.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/9p/vfs_inode.c | 4 ++--
> fs/9p/vfs_inode_dotl.c | 4 ++--
> fs/cifs/inode.c | 5 ++---
> fs/fuse/dir.c | 6 +++---
> fs/fuse/inode.c | 2 +-
> fs/fuse/readdir.c | 2 +-
> fs/nfs/inode.c | 6 +++---
> fs/nfsd/nfsproc.c | 2 +-
> fs/overlayfs/namei.c | 4 ++--
> include/linux/fs.h | 5 +++++
> 10 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 8d97f0b45e9c..795706520b5e 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -399,7 +399,7 @@ static int v9fs_test_inode(struct inode *inode, void *data)
>
>
> umode = p9mode2unixmode(v9ses, st, &rdev);
> /* don't match inode of different type */
> - if ((inode->i_mode & S_IFMT) != (umode & S_IFMT))
> + if (inode_wrong_type(inode, umode))
> return 0;
>
>
> /* compare qid details */
> @@ -1390,7 +1390,7 @@ int v9fs_refresh_inode(struct p9_fid *fid, struct inode *inode)
> * Don't update inode if the file type is different
> */
> umode = p9mode2unixmode(v9ses, st, &rdev);
> - if ((inode->i_mode & S_IFMT) != (umode & S_IFMT))
> + if (inode_wrong_type(inode, umode))
> goto out;
>
>
> /*
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 1dc7af046615..df0b87b05c42 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -59,7 +59,7 @@ static int v9fs_test_inode_dotl(struct inode *inode, void *data)
> struct p9_stat_dotl *st = (struct p9_stat_dotl *)data;
>
>
> /* don't match inode of different type */
> - if ((inode->i_mode & S_IFMT) != (st->st_mode & S_IFMT))
> + if (inode_wrong_type(inode, st->st_mode))
> return 0;
>
>
> if (inode->i_generation != st->st_gen)
> @@ -959,7 +959,7 @@ int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode)
> /*
> * Don't update inode if the file type is different
> */
> - if ((inode->i_mode & S_IFMT) != (st->st_mode & S_IFMT))
> + if (inode_wrong_type(inode, st->st_mode))
> goto out;
>
>
> /*
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 7c61bc9573c0..d46b36d52211 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -426,8 +426,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
> }
>
>
> /* if filetype is different, return error */
> - if (unlikely(((*pinode)->i_mode & S_IFMT) !=
> - (fattr.cf_mode & S_IFMT))) {
> + if (unlikely(inode_wrong_type(*pinode, fattr.cf_mode))) {
> CIFS_I(*pinode)->time = 0; /* force reval */
> rc = -ESTALE;
> goto cgiiu_exit;
> @@ -1249,7 +1248,7 @@ cifs_find_inode(struct inode *inode, void *opaque)
> return 0;
>
>
> /* don't match inode of different type */
> - if ((inode->i_mode & S_IFMT) != (fattr->cf_mode & S_IFMT))
> + if (inode_wrong_type(inode, fattr->cf_mode))
> return 0;
>
>
> /* if it's not a directory or has no dentries, then flag it */
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 06a18700a845..2400b98e8808 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -252,7 +252,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
> if (ret == -ENOMEM)
> goto out;
> if (ret || fuse_invalid_attr(&outarg.attr) ||
> - (outarg.attr.mode ^ inode->i_mode) & S_IFMT)
> + inode_wrong_type(inode, outarg.attr.mode))
> goto invalid;
>
>
> forget_all_cached_acls(inode);
> @@ -1054,7 +1054,7 @@ static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
> err = fuse_simple_request(fm, &args);
> if (!err) {
> if (fuse_invalid_attr(&outarg.attr) ||
> - (inode->i_mode ^ outarg.attr.mode) & S_IFMT) {
> + inode_wrong_type(inode, outarg.attr.mode)) {
> fuse_make_bad(inode);
> err = -EIO;
> } else {
> @@ -1703,7 +1703,7 @@ int fuse_do_setattr(struct dentry *dentry, struct iattr *attr,
> }
>
>
> if (fuse_invalid_attr(&outarg.attr) ||
> - (inode->i_mode ^ outarg.attr.mode) & S_IFMT) {
> + inode_wrong_type(inode, outarg.attr.mode)) {
> fuse_make_bad(inode);
> err = -EIO;
> goto error;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index b0e18b470e91..b4b956da3851 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -350,7 +350,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
> inode->i_generation = generation;
> fuse_init_inode(inode, attr);
> unlock_new_inode(inode);
> - } else if ((inode->i_mode ^ attr->mode) & S_IFMT) {
> + } else if (inode_wrong_type(inode, attr->mode)) {
> /* Inode has changed type, any I/O on the old should fail */
> fuse_make_bad(inode);
> iput(inode);
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index 3441ffa740f3..277f7041d55a 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -202,7 +202,7 @@ static int fuse_direntplus_link(struct file *file,
> inode = d_inode(dentry);
> if (!inode ||
> get_node_id(inode) != o->nodeid ||
> - ((o->attr.mode ^ inode->i_mode) & S_IFMT)) {
> + inode_wrong_type(inode, o->attr.mode)) {
> d_invalidate(dentry);
> dput(dentry);
> goto retry;
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 749bbea14d99..b0da2408816d 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -334,7 +334,7 @@ nfs_find_actor(struct inode *inode, void *opaque)
>
>
> if (NFS_FILEID(inode) != fattr->fileid)
> return 0;
> - if ((S_IFMT & inode->i_mode) != (S_IFMT & fattr->mode))
> + if (inode_wrong_type(inode, fattr->mode))
> return 0;
> if (nfs_compare_fh(NFS_FH(inode), fh))
> return 0;
> @@ -1460,7 +1460,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
> return 0;
> return -ESTALE;
> }
> - if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
> + if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && inode_wrong_type(inode, fattr->mode))
> return -ESTALE;
>
>
>
>
> @@ -1875,7 +1875,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
> /*
> * Make sure the inode's type hasn't changed.
> */
> - if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT)) {
> + if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && inode_wrong_type(inode, fattr->mode)) {
> /*
> * Big trouble! The inode has become a different object.
> */
> diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
> index a8d5449dd0e9..6d51687a0585 100644
> --- a/fs/nfsd/nfsproc.c
> +++ b/fs/nfsd/nfsproc.c
> @@ -381,7 +381,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
>
>
> /* Make sure the type and device matches */
> resp->status = nfserr_exist;
> - if (inode && type != (inode->i_mode & S_IFMT))
> + if (inode && inode_wrong_type(inode, type))
> goto out_unlock;
> }
>
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 3fe05fb5d145..1d573972ce22 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -371,7 +371,7 @@ int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
> return PTR_ERR(origin);
>
>
> if (upperdentry && !ovl_is_whiteout(upperdentry) &&
> - ((d_inode(origin)->i_mode ^ d_inode(upperdentry)->i_mode) & S_IFMT))
> + inode_wrong_type(d_inode(upperdentry), d_inode(origin)->i_mode))
> goto invalid;
>
>
> if (!*stackp)
> @@ -730,7 +730,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
> index = ERR_PTR(-ESTALE);
> goto out;
> } else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
> - ((inode->i_mode ^ d_inode(origin)->i_mode) & S_IFMT)) {
> + inode_wrong_type(inode, d_inode(origin)->i_mode)) {
> /*
> * Index should always be of the same file type as origin
> * except for the case of a whiteout index. A whiteout
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index ec8f3ddf4a6a..9e0d76a41229 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2884,6 +2884,11 @@ static inline bool execute_ok(struct inode *inode)
> return (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode);
> }
>
>
> +static inline bool inode_wrong_type(const struct inode *inode, umode_t mode)
> +{
> + return (inode->i_mode ^ mode) & S_IFMT;
> +}
> +
> static inline void file_start_write(struct file *file)
> {
> if (!S_ISREG(file_inode(file)->i_mode))
I like the new helper -- much easier to read.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread