| Message ID | 20170209142238.5322-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On Feb 9, 2017, at 9:22 AM, Christoph Hellwig <hch@lst.de> wrote: > > The switch to vfs_truncate in nfsd_setattr dropped the owner override > used for NFS permissions. Add a copy of vfs_truncate with it restored > to the nfsd code for now as it's very late in the cycle, but there > should be a way to consolidate it back in the future. > > Fixes: 41f53350 ("nfsd: special case truncates some more") > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reported-by: Chuck Lever <chucklever@gmail.com> > --- > fs/nfsd/vfs.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 80 insertions(+), 1 deletion(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index a974368026a1..fd4a32e0b0b4 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -332,6 +332,85 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap) > } > } > > +/* copy of vfs_truncate with NFS owner override hacked in, sigh.. */ > +static long nfsd_truncate(const struct path *path, loff_t length) > +{ > + struct inode *inode; > + struct dentry *upperdentry; > + long error; > + > + inode = path->dentry->d_inode; > + > + /* For directories it's -EISDIR, for other non-regulars - -EINVAL */ > + if (S_ISDIR(inode->i_mode)) > + return -EISDIR; > + if (!S_ISREG(inode->i_mode)) > + return -EINVAL; > + > + error = mnt_want_write(path->mnt); > + if (error) > + goto out; > + > + /* > + * The file owner always gets access permission for accesses that > + * would normally be checked at open time. This is to make > + * file access work even when the client has done a fchmod(fd, 0). > + * > + * However, `cp foo bar' should fail nevertheless when bar is > + * readonly. A sensible way to do this might be to reject all > + * attempts to truncate a read-only file, because a creat() call > + * always implies file truncation. > + * ... but this isn't really fair. A process may reasonably call > + * ftruncate on an open file descriptor on a file with perm 000. > + * We must trust the client to do permission checking - using "ACCESS" > + * with NFSv3. > + */ > + if (!uid_eq(inode->i_uid, current_fsuid())) { > + error = inode_permission(inode, MAY_WRITE); > + if (error) > + goto mnt_drop_write_and_out; > + } > + > + error = -EPERM; > + if (IS_APPEND(inode)) > + goto mnt_drop_write_and_out; > + > + /* > + * If this is an overlayfs then do as if opening the file so we get > + * write access on the upper inode, not on the overlay inode. For > + * non-overlay filesystems d_real() is an identity function. > + */ > + upperdentry = d_real(path->dentry, NULL, O_WRONLY); > + error = PTR_ERR(upperdentry); > + if (IS_ERR(upperdentry)) > + goto mnt_drop_write_and_out; > + > + error = get_write_access(upperdentry->d_inode); > + if (error) > + goto mnt_drop_write_and_out; > + > + /* > + * Make sure that there are no leases. get_write_access() protects > + * against the truncate racing with a lease-granting setlease(). > + */ > + error = break_lease(inode, O_WRONLY); > + if (error) > + goto put_write_and_out; > + > + error = locks_verify_truncate(inode, NULL, length); > + if (!error) > + error = security_path_truncate(path); > + if (!error) > + error = do_truncate(path->dentry, length, 0, NULL); do_truncate does not seem to be available in the EXPORT_SYMBOL pool. > + > +put_write_and_out: > + put_write_access(upperdentry->d_inode); > +mnt_drop_write_and_out: > + mnt_drop_write(path->mnt); > +out: > + return error; > +} > + > /* > * Set various file attributes. After this call fhp needs an fh_put. > */ > @@ -398,7 +477,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, > ((iap->ia_valid & ~(ATTR_SIZE | ATTR_MTIME)) == 0)) > implicit_mtime = true; > > - host_err = vfs_truncate(&path, iap->ia_size); > + host_err = nfsd_truncate(&path, iap->ia_size); > if (host_err) > goto out_host_err; > > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever chucklever@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Christoph,
[auto build test ERROR on nfsd/nfsd-next]
[also build test ERROR on v4.10-rc7 next-20170209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Christoph-Hellwig/nfsd-restore-owner-override-for-truncate/20170210-035117
base: git://linux-nfs.org/~bfields/linux.git nfsd-next
config: powerpc-mvme5100_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
>> ERROR: "do_truncate" [fs/nfsd/nfsd.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index a974368026a1..fd4a32e0b0b4 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -332,6 +332,85 @@ nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap) } } +/* copy of vfs_truncate with NFS owner override hacked in, sigh.. */ +static long nfsd_truncate(const struct path *path, loff_t length) +{ + struct inode *inode; + struct dentry *upperdentry; + long error; + + inode = path->dentry->d_inode; + + /* For directories it's -EISDIR, for other non-regulars - -EINVAL */ + if (S_ISDIR(inode->i_mode)) + return -EISDIR; + if (!S_ISREG(inode->i_mode)) + return -EINVAL; + + error = mnt_want_write(path->mnt); + if (error) + goto out; + + /* + * The file owner always gets access permission for accesses that + * would normally be checked at open time. This is to make + * file access work even when the client has done a fchmod(fd, 0). + * + * However, `cp foo bar' should fail nevertheless when bar is + * readonly. A sensible way to do this might be to reject all + * attempts to truncate a read-only file, because a creat() call + * always implies file truncation. + * ... but this isn't really fair. A process may reasonably call + * ftruncate on an open file descriptor on a file with perm 000. + * We must trust the client to do permission checking - using "ACCESS" + * with NFSv3. + */ + if (!uid_eq(inode->i_uid, current_fsuid())) { + error = inode_permission(inode, MAY_WRITE); + if (error) + goto mnt_drop_write_and_out; + } + + error = -EPERM; + if (IS_APPEND(inode)) + goto mnt_drop_write_and_out; + + /* + * If this is an overlayfs then do as if opening the file so we get + * write access on the upper inode, not on the overlay inode. For + * non-overlay filesystems d_real() is an identity function. + */ + upperdentry = d_real(path->dentry, NULL, O_WRONLY); + error = PTR_ERR(upperdentry); + if (IS_ERR(upperdentry)) + goto mnt_drop_write_and_out; + + error = get_write_access(upperdentry->d_inode); + if (error) + goto mnt_drop_write_and_out; + + /* + * Make sure that there are no leases. get_write_access() protects + * against the truncate racing with a lease-granting setlease(). + */ + error = break_lease(inode, O_WRONLY); + if (error) + goto put_write_and_out; + + error = locks_verify_truncate(inode, NULL, length); + if (!error) + error = security_path_truncate(path); + if (!error) + error = do_truncate(path->dentry, length, 0, NULL); + +put_write_and_out: + put_write_access(upperdentry->d_inode); +mnt_drop_write_and_out: + mnt_drop_write(path->mnt); +out: + return error; +} + /* * Set various file attributes. After this call fhp needs an fh_put. */ @@ -398,7 +477,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, ((iap->ia_valid & ~(ATTR_SIZE | ATTR_MTIME)) == 0)) implicit_mtime = true; - host_err = vfs_truncate(&path, iap->ia_size); + host_err = nfsd_truncate(&path, iap->ia_size); if (host_err) goto out_host_err;
The switch to vfs_truncate in nfsd_setattr dropped the owner override used for NFS permissions. Add a copy of vfs_truncate with it restored to the nfsd code for now as it's very late in the cycle, but there should be a way to consolidate it back in the future. Fixes: 41f53350 ("nfsd: special case truncates some more") Signed-off-by: Christoph Hellwig <hch@lst.de> Reported-by: Chuck Lever <chucklever@gmail.com> --- fs/nfsd/vfs.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-)