Re: [PATCH] nfsd: Return EPERM on utime/utimes/lutimes calls instead of EACCESS in some cases

J. Bruce Fields
2018-12-04T20:13:59Z

On Mon, Dec 03, 2018 at 04:36:27PM +0800, zhengbin (A) wrote:
> hi, J. Bruce Fields, Jeff Layton
>
> When you have time, confirm it? thanks

Sorry for the delay.

The patch looks good to me.

As far as I can tell, we've done this forever. And I'm a little worried
about changing such long-standing behavior, but I think you're right.

So, applying for for 4.21.

--b.

> On 2018/11/30 15:51, zhengbin wrote:
> > As the man(3) page for utime/utimes/lutimes, EPERM is returned
> > when the second parameter of utime/utimes/lutimes is not NULL,
> > the caller's effective UID does not match the owner of the file,
> > and the caller is not privileged.
> >
> > However, in a NFS directory, it will return EACCESS(nfsd_setattr->
> > fh_verify->nfsd_permission), This patch fix this.
> >
> > Signed-off-by: zhengbin <zhengbin13@huawei.com>
> > ---
> > fs/nfsd/vfs.c | 17 +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index eb67098..9824e32 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -396,10 +396,23 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
> > bool get_write_count;
> > bool size_change = (iap->ia_valid & ATTR_SIZE);
> >
> > - if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME | ATTR_SIZE))
> > + if (iap->ia_valid & ATTR_SIZE) {
> > accmode |= NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE;
> > - if (iap->ia_valid & ATTR_SIZE)
> > ftype = S_IFREG;
> > + }
> > +
> > + /*
> > + * If utimes(2) and friends are called with times not NULL, we should
> > + * not set NFSD_MAY_WRITE bit. Otherwise fh_verify->nfsd_permission
> > + * will return EACCESS, when the caller's effective UID does not match
> > + * the owner of the file, and the caller is not privileged. In this
> > + * situation, we should return EPERM(notify_change will return this).
> > + */
> > + if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME)) {
> > + accmode |= NFSD_MAY_OWNER_OVERRIDE;
> > + if (!(iap->ia_valid & (ATTR_ATIME_SET | ATTR_MTIME_SET)))
> > + accmode |= NFSD_MAY_WRITE;
> > + }
> >
> > /* Callers that do fh_verify should do the fh_want_write: */
> > get_write_count = !fhp->fh_dentry;
> > --
> > 2.7.4
> >
> >
> > .
> >