> On May 5, 2023, at 4:11 AM, Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> wrote: > > For path-based LSMs such as Landlock, struct path instead of struct > dentry is required to make sense of attr/xattr accesses. notify_change() > is the main caller of security_inode_setattr(), so refactor it first > before lsm hook inode_setattr(). > > This patch also touches do_truncate() and other related callers. > > Signed-off-by: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> > --- > drivers/base/devtmpfs.c | 5 +++-- > fs/attr.c | 5 +++-- > fs/cachefiles/interface.c | 4 ++-- > fs/coredump.c | 2 +- > fs/ecryptfs/inode.c | 18 +++++++++--------- > fs/inode.c | 8 +++++--- > fs/ksmbd/smb2pdu.c | 6 +++--- > fs/ksmbd/smbacl.c | 2 +- > fs/namei.c | 2 +- > fs/nfsd/vfs.c | 12 ++++++++---- > fs/open.c | 19 ++++++++++--------- > fs/overlayfs/overlayfs.h | 4 +++- > fs/utimes.c | 2 +- > include/linux/fs.h | 4 ++-- > 14 files changed, 52 insertions(+), 41 deletions(-) > > diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c > index b848764ef018..e67dd258984c 100644 > --- a/drivers/base/devtmpfs.c > +++ b/drivers/base/devtmpfs.c > @@ -220,13 +220,14 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid, > dev->devt); > if (!err) { > struct iattr newattrs; > + struct path new = { .mnt = path.mnt, .dentry = dentry }; > > newattrs.ia_mode = mode; > newattrs.ia_uid = uid; > newattrs.ia_gid = gid; > newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID; > inode_lock(d_inode(dentry)); > - notify_change(&nop_mnt_idmap, dentry, &newattrs, NULL); > + notify_change(&nop_mnt_idmap, &new, &newattrs, NULL); > inode_unlock(d_inode(dentry)); > > /* mark as kernel-created inode */ > @@ -334,7 +335,7 @@ static int handle_remove(const char *nodename, struct device *dev) > newattrs.ia_valid = > ATTR_UID|ATTR_GID|ATTR_MODE; > inode_lock(d_inode(dentry)); > - notify_change(&nop_mnt_idmap, dentry, &newattrs, NULL); > + notify_change(&nop_mnt_idmap, &p, &newattrs, NULL); > inode_unlock(d_inode(dentry)); > err = vfs_unlink(&nop_mnt_idmap, d_inode(parent.dentry), > dentry, NULL); > diff --git a/fs/attr.c b/fs/attr.c > index d60dc1edb526..eecd78944b83 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -354,7 +354,7 @@ EXPORT_SYMBOL(may_setattr); > /** > * notify_change - modify attributes of a filesytem object > * @idmap: idmap of the mount the inode was found from > - * @dentry: object affected > + * @path: path of object affected > * @attr: new attributes > * @delegated_inode: returns inode, if the inode is delegated > * > @@ -378,9 +378,10 @@ EXPORT_SYMBOL(may_setattr); > * permissions. On non-idmapped mounts or if permission checking is to be > * performed on the raw inode simply pass @nop_mnt_idmap. > */ > -int notify_change(struct mnt_idmap *idmap, struct dentry *dentry, > +int notify_change(struct mnt_idmap *idmap, const struct path *path, > struct iattr *attr, struct inode **delegated_inode) > { > + struct dentry *dentry = path->dentry; > struct inode *inode = dentry->d_inode; > umode_t mode = inode->i_mode; > int error; > diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c > index 40052bdb3365..4700285a76f0 100644 > --- a/fs/cachefiles/interface.c > +++ b/fs/cachefiles/interface.c > @@ -138,7 +138,7 @@ static int cachefiles_adjust_size(struct cachefiles_object *object) > newattrs.ia_size = oi_size & PAGE_MASK; > ret = cachefiles_inject_remove_error(); > if (ret == 0) > - ret = notify_change(&nop_mnt_idmap, file->f_path.dentry, > + ret = notify_change(&nop_mnt_idmap, &file->f_path, > &newattrs, NULL); > if (ret < 0) > goto truncate_failed; > @@ -148,7 +148,7 @@ static int cachefiles_adjust_size(struct cachefiles_object *object) > newattrs.ia_size = ni_size; > ret = cachefiles_inject_write_error(); > if (ret == 0) > - ret = notify_change(&nop_mnt_idmap, file->f_path.dentry, > + ret = notify_change(&nop_mnt_idmap, &file->f_path, > &newattrs, NULL); > > truncate_failed: > diff --git a/fs/coredump.c b/fs/coredump.c > index ece7badf701b..01bef4830bfa 100644 > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -736,7 +736,7 @@ void do_coredump(const kernel_siginfo_t *siginfo) > } > if (!(cprm.file->f_mode & FMODE_CAN_WRITE)) > goto close_fail; > - if (do_truncate(idmap, cprm.file->f_path.dentry, > + if (do_truncate(idmap, &cprm.file->f_path, > 0, 0, cprm.file)) > goto close_fail; > } > diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c > index 83274915ba6d..423bd457623e 100644 > --- a/fs/ecryptfs/inode.c > +++ b/fs/ecryptfs/inode.c > @@ -853,12 +853,12 @@ int ecryptfs_truncate(struct dentry *dentry, loff_t new_length) > > rc = truncate_upper(dentry, &ia, &lower_ia); > if (!rc && lower_ia.ia_valid & ATTR_SIZE) { > - struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry); > + const struct path *lower_path = ecryptfs_dentry_to_lower_path(dentry); > > - inode_lock(d_inode(lower_dentry)); > - rc = notify_change(&nop_mnt_idmap, lower_dentry, > + inode_lock(d_inode(lower_path->dentry)); > + rc = notify_change(&nop_mnt_idmap, lower_path, > &lower_ia, NULL); > - inode_unlock(d_inode(lower_dentry)); > + inode_unlock(d_inode(lower_path->dentry)); > } > return rc; > } > @@ -888,7 +888,7 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap, > struct dentry *dentry, struct iattr *ia) > { > int rc = 0; > - struct dentry *lower_dentry; > + const struct path *lower_path; > struct iattr lower_ia; > struct inode *inode; > struct inode *lower_inode; > @@ -902,7 +902,7 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap, > } > inode = d_inode(dentry); > lower_inode = ecryptfs_inode_to_lower(inode); > - lower_dentry = ecryptfs_dentry_to_lower(dentry); > + lower_path = ecryptfs_dentry_to_lower_path(dentry); > mutex_lock(&crypt_stat->cs_mutex); > if (d_is_dir(dentry)) > crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED); > @@ -964,9 +964,9 @@ static int ecryptfs_setattr(struct mnt_idmap *idmap, > if (lower_ia.ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) > lower_ia.ia_valid &= ~ATTR_MODE; > > - inode_lock(d_inode(lower_dentry)); > - rc = notify_change(&nop_mnt_idmap, lower_dentry, &lower_ia, NULL); > - inode_unlock(d_inode(lower_dentry)); > + inode_lock(d_inode(lower_path->dentry)); > + rc = notify_change(&nop_mnt_idmap, lower_path, &lower_ia, NULL); > + inode_unlock(d_inode(lower_path->dentry)); > out: > fsstack_copy_attr_all(inode, lower_inode); > return rc; > diff --git a/fs/inode.c b/fs/inode.c > index 577799b7855f..4fa51d46f655 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -1973,7 +1973,7 @@ int dentry_needs_remove_privs(struct mnt_idmap *idmap, > } > > static int __remove_privs(struct mnt_idmap *idmap, > - struct dentry *dentry, int kill) > + struct path *path, int kill) > { > struct iattr newattrs; > > @@ -1982,13 +1982,15 @@ static int __remove_privs(struct mnt_idmap *idmap, > * Note we call this on write, so notify_change will not > * encounter any conflicting delegations: > */ > - return notify_change(idmap, dentry, &newattrs, NULL); > + return notify_change(idmap, path, &newattrs, NULL); > } > > static int __file_remove_privs(struct file *file, unsigned int flags) > { > struct dentry *dentry = file_dentry(file); > struct inode *inode = file_inode(file); > + /* this path maybe incorrect */ > + struct path path = {.mnt = file->f_path.mnt, .dentry = dentry}; > int error = 0; > int kill; > > @@ -2003,7 +2005,7 @@ static int __file_remove_privs(struct file *file, unsigned int flags) > if (flags & IOCB_NOWAIT) > return -EAGAIN; > > - error = __remove_privs(file_mnt_idmap(file), dentry, kill); > + error = __remove_privs(file_mnt_idmap(file), &path, kill); > } > > if (!error) > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index cb93fd231f4e..2b7e5c446397 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -5644,8 +5644,8 @@ static int set_file_basic_info(struct ksmbd_file *fp, > } > > if (attrs.ia_valid) { > - struct dentry *dentry = filp->f_path.dentry; > - struct inode *inode = d_inode(dentry); > + struct path *path = &filp->f_path; > + struct inode *inode = d_inode(path->dentry); > > if (IS_IMMUTABLE(inode) || IS_APPEND(inode)) > return -EACCES; > @@ -5653,7 +5653,7 @@ static int set_file_basic_info(struct ksmbd_file *fp, > inode_lock(inode); > inode->i_ctime = attrs.ia_ctime; > attrs.ia_valid &= ~ATTR_CTIME; > - rc = notify_change(idmap, dentry, &attrs, NULL); > + rc = notify_change(idmap, path, &attrs, NULL); > inode_unlock(inode); > } > return rc; > diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c > index 6d6cfb6957a9..39d8aff3ae1b 100644 > --- a/fs/ksmbd/smbacl.c > +++ b/fs/ksmbd/smbacl.c > @@ -1403,7 +1403,7 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon, > } > > inode_lock(inode); > - rc = notify_change(idmap, path->dentry, &newattrs, NULL); > + rc = notify_change(idmap, path, &newattrs, NULL); > inode_unlock(inode); > if (rc) > goto out; > diff --git a/fs/namei.c b/fs/namei.c > index e4fe0879ae55..ec7075a8505d 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -3292,7 +3292,7 @@ static int handle_truncate(struct mnt_idmap *idmap, struct file *filp) > > error = security_file_truncate(filp); > if (!error) { > - error = do_truncate(idmap, path->dentry, 0, > + error = do_truncate(idmap, path, 0, > ATTR_MTIME|ATTR_CTIME|ATTR_OPEN, > filp); > } > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index bb9d47172162..4b51fd2f05e3 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -410,7 +410,7 @@ nfsd_get_write_access(struct svc_rqst *rqstp, struct svc_fh *fhp, > return nfserrno(get_write_access(inode)); > } > > -static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap) > +static int __nfsd_setattr(struct path *path, struct iattr *iap) > { > int host_err; > > @@ -430,7 +430,7 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap) > if (iap->ia_size < 0) > return -EFBIG; > > - host_err = notify_change(&nop_mnt_idmap, dentry, &size_attr, NULL); > + host_err = notify_change(&nop_mnt_idmap, path, &size_attr, NULL); > if (host_err) > return host_err; > iap->ia_valid &= ~ATTR_SIZE; > @@ -448,7 +448,7 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap) > return 0; > > iap->ia_valid |= ATTR_CTIME; > - return notify_change(&nop_mnt_idmap, dentry, iap, NULL); > + return notify_change(&nop_mnt_idmap, path, iap, NULL); > } > > /** > @@ -471,6 +471,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, > struct nfsd_attrs *attr, > int check_guard, time64_t guardtime) > { > + struct path path; > struct dentry *dentry; > struct inode *inode; > struct iattr *iap = attr->na_iattr; > @@ -534,9 +535,12 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, > return err; > } > > + path.mnt = fhp->fh_export->ex_path.mnt; > + path.dentry = fhp->fh_dentry; > + > inode_lock(inode); > for (retries = 1;;) { > - host_err = __nfsd_setattr(dentry, iap); > + host_err = __nfsd_setattr(&path, iap); > if (host_err != -EAGAIN || !retries--) > break; > if (!nfsd_wait_for_delegreturn(rqstp, inode)) Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx <mailto:chuck.lever@xxxxxxxxxx>> > diff --git a/fs/open.c b/fs/open.c > index 4478adcc4f3a..7a7841606285 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -37,11 +37,12 @@ > > #include "internal.h" > > -int do_truncate(struct mnt_idmap *idmap, struct dentry *dentry, > +int do_truncate(struct mnt_idmap *idmap, const struct path *path, > loff_t length, unsigned int time_attrs, struct file *filp) > { > int ret; > struct iattr newattrs; > + struct dentry *dentry = path->dentry; > > /* Not pretty: "inode->i_size" shouldn't really be signed. But it is. */ > if (length < 0) > @@ -63,7 +64,7 @@ int do_truncate(struct mnt_idmap *idmap, struct dentry *dentry, > > inode_lock(dentry->d_inode); > /* Note any delegations or leases have already been broken: */ > - ret = notify_change(idmap, dentry, &newattrs, NULL); > + ret = notify_change(idmap, path, &newattrs, NULL); > inode_unlock(dentry->d_inode); > return ret; > } > @@ -109,7 +110,7 @@ long vfs_truncate(const struct path *path, loff_t length) > > error = security_path_truncate(path); > if (!error) > - error = do_truncate(idmap, path->dentry, length, 0, NULL); > + error = do_truncate(idmap, path, length, 0, NULL); > > put_write_and_out: > put_write_access(inode); > @@ -157,7 +158,7 @@ COMPAT_SYSCALL_DEFINE2(truncate, const char __user *, path, compat_off_t, length > long do_sys_ftruncate(unsigned int fd, loff_t length, int small) > { > struct inode *inode; > - struct dentry *dentry; > + struct path *path; > struct fd f; > int error; > > @@ -173,8 +174,8 @@ long do_sys_ftruncate(unsigned int fd, loff_t length, int small) > if (f.file->f_flags & O_LARGEFILE) > small = 0; > > - dentry = f.file->f_path.dentry; > - inode = dentry->d_inode; > + path = &f.file->f_path; > + inode = d_inode(path->dentry); > error = -EINVAL; > if (!S_ISREG(inode->i_mode) || !(f.file->f_mode & FMODE_WRITE)) > goto out_putf; > @@ -191,7 +192,7 @@ long do_sys_ftruncate(unsigned int fd, loff_t length, int small) > sb_start_write(inode->i_sb); > error = security_file_truncate(f.file); > if (!error) > - error = do_truncate(file_mnt_idmap(f.file), dentry, length, > + error = do_truncate(file_mnt_idmap(f.file), path, length, > ATTR_MTIME | ATTR_CTIME, f.file); > sb_end_write(inode->i_sb); > out_putf: > @@ -640,7 +641,7 @@ int chmod_common(const struct path *path, umode_t mode) > goto out_unlock; > newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO); > newattrs.ia_valid = ATTR_MODE | ATTR_CTIME; > - error = notify_change(mnt_idmap(path->mnt), path->dentry, > + error = notify_change(mnt_idmap(path->mnt), path, > &newattrs, &delegated_inode); > out_unlock: > inode_unlock(inode); > @@ -771,7 +772,7 @@ int chown_common(const struct path *path, uid_t user, gid_t group) > from_vfsuid(idmap, fs_userns, newattrs.ia_vfsuid), > from_vfsgid(idmap, fs_userns, newattrs.ia_vfsgid)); > if (!error) > - error = notify_change(idmap, path->dentry, &newattrs, > + error = notify_change(idmap, path, &newattrs, > &delegated_inode); > inode_unlock(inode); > if (delegated_inode) { > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > index 4d0b278f5630..d1a1eaa1c00c 100644 > --- a/fs/overlayfs/overlayfs.h > +++ b/fs/overlayfs/overlayfs.h > @@ -141,7 +141,9 @@ static inline int ovl_do_notify_change(struct ovl_fs *ofs, > struct dentry *upperdentry, > struct iattr *attr) > { > - return notify_change(ovl_upper_mnt_idmap(ofs), upperdentry, attr, NULL); > + struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = upperdentry }; > + > + return notify_change(ovl_upper_mnt_idmap(ofs), &path, attr, NULL); > } > > static inline int ovl_do_rmdir(struct ovl_fs *ofs, > diff --git a/fs/utimes.c b/fs/utimes.c > index 3701b3946f88..1e6b82b27899 100644 > --- a/fs/utimes.c > +++ b/fs/utimes.c > @@ -63,7 +63,7 @@ int vfs_utimes(const struct path *path, struct timespec64 *times) > } > retry_deleg: > inode_lock(inode); > - error = notify_change(mnt_idmap(path->mnt), path->dentry, &newattrs, > + error = notify_change(mnt_idmap(path->mnt), path, &newattrs, > &delegated_inode); > inode_unlock(inode); > if (delegated_inode) { > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 21a981680856..dbba36ab4a1b 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2333,7 +2333,7 @@ static inline bool is_idmapped_mnt(const struct vfsmount *mnt) > } > > extern long vfs_truncate(const struct path *, loff_t); > -int do_truncate(struct mnt_idmap *, struct dentry *, loff_t start, > +int do_truncate(struct mnt_idmap *, const struct path *, loff_t start, > unsigned int time_attrs, struct file *filp); > extern int vfs_fallocate(struct file *file, int mode, loff_t offset, > loff_t len); > @@ -2488,7 +2488,7 @@ static inline int bmap(struct inode *inode, sector_t *block) > } > #endif > > -int notify_change(struct mnt_idmap *, struct dentry *, > +int notify_change(struct mnt_idmap *, const struct path *, > struct iattr *, struct inode **); > int inode_permission(struct mnt_idmap *, struct inode *, int); > int generic_permission(struct mnt_idmap *, struct inode *, int); > -- > 2.17.1 > -- Chuck Lever -- Linux-cachefs mailing list Linux-cachefs@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/linux-cachefs