From: "J. Bruce Fields" <[email protected]>
We're currently ignoring errors from vfs_getattr.
The correct thing to do is to do the stat in the main service procedure
not in the response encoding.
Reported-by: Al Viro <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs2acl.c | 31 +++++++++++++++++++++++++++----
fs/nfsd/nfsxdr.c | 6 ++----
fs/nfsd/xdr.h | 2 +-
fs/nfsd/xdr3.h | 2 ++
4 files changed, 32 insertions(+), 9 deletions(-)
For 3.9.
diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 9170861..b99864b 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -24,6 +24,14 @@ nfsacld_proc_null(struct svc_rqst *rqstp, void *argp, void *resp)
return nfs_ok;
}
+static __be32 nfsd_getattr(struct svc_fh *fh, struct kstat *stat)
+{
+ int err;
+
+ err = vfs_getattr(fh->fh_export->ex_path.mnt, fh->fh_dentry, stat);
+ return nfserrno(err);
+}
+
/*
* Get the Access and/or Default ACL of a file.
*/
@@ -45,6 +53,10 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst * rqstp,
RETURN_STATUS(nfserr_inval);
resp->mask = argp->mask;
+ nfserr = nfsd_getattr(fh, &resp->stat);
+ if (nfserr)
+ goto fail;
+
if (resp->mask & (NFS_ACL|NFS_ACLCNT)) {
acl = nfsd_get_posix_acl(fh, ACL_TYPE_ACCESS);
if (IS_ERR(acl)) {
@@ -115,6 +127,9 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
nfserr = nfserrno( nfsd_set_posix_acl(
fh, ACL_TYPE_DEFAULT, argp->acl_default) );
}
+ if (!nfserr) {
+ nfserr = nfsd_getattr(fh, &resp->stat);
+ }
/* argp->acl_{access,default} may have been allocated in
nfssvc_decode_setaclargs. */
@@ -129,10 +144,15 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
static __be32 nfsacld_proc_getattr(struct svc_rqst * rqstp,
struct nfsd_fhandle *argp, struct nfsd_attrstat *resp)
{
+ __be32 nfserr;
dprintk("nfsd: GETATTR %s\n", SVCFH_fmt(&argp->fh));
fh_copy(&resp->fh, &argp->fh);
- return fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+ nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+ if (nfserr)
+ return nfserr;
+ nfserr = nfsd_getattr(&resp->fh, &resp->stat);
+ return nfserr;
}
/*
@@ -150,6 +170,9 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp, struct nfsd3_accessarg
fh_copy(&resp->fh, &argp->fh);
resp->access = argp->access;
nfserr = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
+ if (nfserr)
+ return nfserr;
+ nfserr = nfsd_getattr(&resp->fh, &resp->stat);
return nfserr;
}
@@ -243,7 +266,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
return 0;
inode = dentry->d_inode;
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
*p++ = htonl(resp->mask);
if (!xdr_ressize_check(rqstp, p))
return 0;
@@ -274,7 +297,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
struct nfsd_attrstat *resp)
{
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
return xdr_ressize_check(rqstp, p);
}
@@ -282,7 +305,7 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p,
struct nfsd3_accessres *resp)
{
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
*p++ = htonl(resp->access);
return xdr_ressize_check(rqstp, p);
}
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 979b421..90f47fe 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -194,11 +194,9 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
}
/* Helper function for NFSv2 ACL code */
-__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
+__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat)
{
- struct kstat stat;
- vfs_getattr(fhp->fh_export->ex_path.mnt, fhp->fh_dentry, &stat);
- return encode_fattr(rqstp, p, fhp, &stat);
+ return encode_fattr(rqstp, p, fhp, stat);
}
/*
diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
index 53b1863..4f0481d 100644
--- a/fs/nfsd/xdr.h
+++ b/fs/nfsd/xdr.h
@@ -167,7 +167,7 @@ int nfssvc_encode_entry(void *, const char *name,
int nfssvc_release_fhandle(struct svc_rqst *, __be32 *, struct nfsd_fhandle *);
/* Helper functions for NFSv2 ACL code */
-__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp);
+__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat);
__be32 *nfs2svc_decode_fh(__be32 *p, struct svc_fh *fhp);
#endif /* LINUX_NFSD_H */
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 7df980e..b6d5542 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -136,6 +136,7 @@ struct nfsd3_accessres {
__be32 status;
struct svc_fh fh;
__u32 access;
+ struct kstat stat;
};
struct nfsd3_readlinkres {
@@ -225,6 +226,7 @@ struct nfsd3_getaclres {
int mask;
struct posix_acl *acl_access;
struct posix_acl *acl_default;
+ struct kstat stat;
};
/* dummy type for release */
--
1.7.9.5
On Fri, Feb 22, 2013 at 05:18:35PM -0500, J. Bruce Fields wrote:
> > FWIW, I'm going to push the first part of VFS queue later tonight...
>
> OK, I'll wait and see what's in that.
See for-next...
> (And what should I do with the delegation patches?)
Looking through that stuff...
On Fri, Feb 01, 2013 at 03:13:04PM -0500, J. Bruce Fields wrote:
> On Fri, Feb 01, 2013 at 06:57:22PM +0000, Al Viro wrote:
> > On Fri, Feb 01, 2013 at 08:15:37AM -0500, J. Bruce Fields wrote:
> > > From: "J. Bruce Fields" <[email protected]>
> > >
> > > We're currently ignoring errors from vfs_getattr.
> > >
> > > The correct thing to do is to do the stat in the main service procedure
> > > not in the response encoding.
> >
> > FWIW, I'd combine that with parts of commit I've got in my tree; I think
> > nfsd_getattr() in your variant isn't the best API here. All callers
> > that want nfserrno want vfsmount/dentry coming from some fhandle. Diff
> > below is introducing such helper and switching to its use (warning: edited
> > patch; only obvious editing done there, but still). It does *not* address
> > the issue your patch deals with (see /* BUG */ in there), but I really
> > think it's a better interface than your variant.
>
> Actually, we have precisely the same interface except for the name:
>
> __be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
> vs.
> __be32 nfsd_getattr(struct svc_fh *fh, struct kstat *stat)
>
> but I'm fine with your name.
>
> Do you want to take these patches, or should I?
I guess what I'll do unless I hear otherwise is apply both your patch
and mine to my tree for 3.9.
--b.
>
> Assuming the latter: this is a version of my bugfix rebased on top of
> what you just sent me. I did a quick test and verified it doesn't crash
> when I asked for an acl....
>
> --b.
>
> commit 7afea2b2cd951c3a566356206d84a0f5b8aa0e1a
> Author: J. Bruce Fields <[email protected]>
> Date: Wed Jan 30 16:10:19 2013 -0500
>
> nfsd: handle vfs_getattr errors in acl protocol
>
> We're currently ignoring errors from vfs_getattr.
>
> The correct thing to do is to do the stat in the main service procedure
> not in the response encoding.
>
> Reported-by: Al Viro <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
>
> diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
> index 9170861..95d76dc 100644
> --- a/fs/nfsd/nfs2acl.c
> +++ b/fs/nfsd/nfs2acl.c
> @@ -45,6 +45,10 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst * rqstp,
> RETURN_STATUS(nfserr_inval);
> resp->mask = argp->mask;
>
> + nfserr = fh_getattr(fh, &resp->stat);
> + if (nfserr)
> + goto fail;
> +
> if (resp->mask & (NFS_ACL|NFS_ACLCNT)) {
> acl = nfsd_get_posix_acl(fh, ACL_TYPE_ACCESS);
> if (IS_ERR(acl)) {
> @@ -115,6 +119,9 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
> nfserr = nfserrno( nfsd_set_posix_acl(
> fh, ACL_TYPE_DEFAULT, argp->acl_default) );
> }
> + if (!nfserr) {
> + nfserr = fh_getattr(fh, &resp->stat);
> + }
>
> /* argp->acl_{access,default} may have been allocated in
> nfssvc_decode_setaclargs. */
> @@ -129,10 +136,15 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
> static __be32 nfsacld_proc_getattr(struct svc_rqst * rqstp,
> struct nfsd_fhandle *argp, struct nfsd_attrstat *resp)
> {
> + __be32 nfserr;
> dprintk("nfsd: GETATTR %s\n", SVCFH_fmt(&argp->fh));
>
> fh_copy(&resp->fh, &argp->fh);
> - return fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
> + nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
> + if (nfserr)
> + return nfserr;
> + nfserr = fh_getattr(&resp->fh, &resp->stat);
> + return nfserr;
> }
>
> /*
> @@ -150,6 +162,9 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp, struct nfsd3_accessarg
> fh_copy(&resp->fh, &argp->fh);
> resp->access = argp->access;
> nfserr = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
> + if (nfserr)
> + return nfserr;
> + nfserr = fh_getattr(&resp->fh, &resp->stat);
> return nfserr;
> }
>
> @@ -243,7 +258,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
> return 0;
> inode = dentry->d_inode;
>
> - p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
> + p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
> *p++ = htonl(resp->mask);
> if (!xdr_ressize_check(rqstp, p))
> return 0;
> @@ -274,7 +289,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
> static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
> struct nfsd_attrstat *resp)
> {
> - p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
> + p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
> return xdr_ressize_check(rqstp, p);
> }
>
> @@ -282,7 +297,7 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
> static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p,
> struct nfsd3_accessres *resp)
> {
> - p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
> + p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
> *p++ = htonl(resp->access);
> return xdr_ressize_check(rqstp, p);
> }
> diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
> index bf6d3bc..96e5619 100644
> --- a/fs/nfsd/nfsxdr.c
> +++ b/fs/nfsd/nfsxdr.c
> @@ -195,11 +195,9 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
> }
>
> /* Helper function for NFSv2 ACL code */
> -__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
> +__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat)
> {
> - struct kstat stat;
> - fh_getattr(fhp, &stat); /* BUG */
> - return encode_fattr(rqstp, p, fhp, &stat);
> + return encode_fattr(rqstp, p, fhp, stat);
> }
>
> /*
> diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
> index 53b1863..4f0481d 100644
> --- a/fs/nfsd/xdr.h
> +++ b/fs/nfsd/xdr.h
> @@ -167,7 +167,7 @@ int nfssvc_encode_entry(void *, const char *name,
> int nfssvc_release_fhandle(struct svc_rqst *, __be32 *, struct nfsd_fhandle *);
>
> /* Helper functions for NFSv2 ACL code */
> -__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp);
> +__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat);
> __be32 *nfs2svc_decode_fh(__be32 *p, struct svc_fh *fhp);
>
> #endif /* LINUX_NFSD_H */
> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
> index 7df980e..b6d5542 100644
> --- a/fs/nfsd/xdr3.h
> +++ b/fs/nfsd/xdr3.h
> @@ -136,6 +136,7 @@ struct nfsd3_accessres {
> __be32 status;
> struct svc_fh fh;
> __u32 access;
> + struct kstat stat;
> };
>
> struct nfsd3_readlinkres {
> @@ -225,6 +226,7 @@ struct nfsd3_getaclres {
> int mask;
> struct posix_acl *acl_access;
> struct posix_acl *acl_default;
> + struct kstat stat;
> };
>
> /* dummy type for release */
On Fri, Feb 22, 2013 at 10:15:06PM +0000, Al Viro wrote:
> On Fri, Feb 22, 2013 at 04:46:34PM -0500, J. Bruce Fields wrote:
>
> > > Actually, we have precisely the same interface except for the name:
> > >
> > > __be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
> > > vs.
> > > __be32 nfsd_getattr(struct svc_fh *fh, struct kstat *stat)
> > >
> > > but I'm fine with your name.
> > >
> > > Do you want to take these patches, or should I?
>
> *blink*
>
> I blame being very low on caffeine; that, or being a blind idiot...
> My apologies ;-/
>
> > I guess what I'll do unless I hear otherwise is apply both your patch
> > and mine to my tree for 3.9.
>
> FWIW, I'm going to push the first part of VFS queue later tonight...
OK, I'll wait and see what's in that.
(And what should I do with the delegation patches?)
--b.
On Fri, Feb 01, 2013 at 06:57:22PM +0000, Al Viro wrote:
> On Fri, Feb 01, 2013 at 08:15:37AM -0500, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > We're currently ignoring errors from vfs_getattr.
> >
> > The correct thing to do is to do the stat in the main service procedure
> > not in the response encoding.
>
> FWIW, I'd combine that with parts of commit I've got in my tree; I think
> nfsd_getattr() in your variant isn't the best API here. All callers
> that want nfserrno want vfsmount/dentry coming from some fhandle. Diff
> below is introducing such helper and switching to its use (warning: edited
> patch; only obvious editing done there, but still). It does *not* address
> the issue your patch deals with (see /* BUG */ in there), but I really
> think it's a better interface than your variant.
Actually, we have precisely the same interface except for the name:
__be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
vs.
__be32 nfsd_getattr(struct svc_fh *fh, struct kstat *stat)
but I'm fine with your name.
Do you want to take these patches, or should I?
Assuming the latter: this is a version of my bugfix rebased on top of
what you just sent me. I did a quick test and verified it doesn't crash
when I asked for an acl....
--b.
commit 7afea2b2cd951c3a566356206d84a0f5b8aa0e1a
Author: J. Bruce Fields <[email protected]>
Date: Wed Jan 30 16:10:19 2013 -0500
nfsd: handle vfs_getattr errors in acl protocol
We're currently ignoring errors from vfs_getattr.
The correct thing to do is to do the stat in the main service procedure
not in the response encoding.
Reported-by: Al Viro <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 9170861..95d76dc 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -45,6 +45,10 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst * rqstp,
RETURN_STATUS(nfserr_inval);
resp->mask = argp->mask;
+ nfserr = fh_getattr(fh, &resp->stat);
+ if (nfserr)
+ goto fail;
+
if (resp->mask & (NFS_ACL|NFS_ACLCNT)) {
acl = nfsd_get_posix_acl(fh, ACL_TYPE_ACCESS);
if (IS_ERR(acl)) {
@@ -115,6 +119,9 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
nfserr = nfserrno( nfsd_set_posix_acl(
fh, ACL_TYPE_DEFAULT, argp->acl_default) );
}
+ if (!nfserr) {
+ nfserr = fh_getattr(fh, &resp->stat);
+ }
/* argp->acl_{access,default} may have been allocated in
nfssvc_decode_setaclargs. */
@@ -129,10 +136,15 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
static __be32 nfsacld_proc_getattr(struct svc_rqst * rqstp,
struct nfsd_fhandle *argp, struct nfsd_attrstat *resp)
{
+ __be32 nfserr;
dprintk("nfsd: GETATTR %s\n", SVCFH_fmt(&argp->fh));
fh_copy(&resp->fh, &argp->fh);
- return fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+ nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+ if (nfserr)
+ return nfserr;
+ nfserr = fh_getattr(&resp->fh, &resp->stat);
+ return nfserr;
}
/*
@@ -150,6 +162,9 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp, struct nfsd3_accessarg
fh_copy(&resp->fh, &argp->fh);
resp->access = argp->access;
nfserr = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
+ if (nfserr)
+ return nfserr;
+ nfserr = fh_getattr(&resp->fh, &resp->stat);
return nfserr;
}
@@ -243,7 +258,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
return 0;
inode = dentry->d_inode;
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
*p++ = htonl(resp->mask);
if (!xdr_ressize_check(rqstp, p))
return 0;
@@ -274,7 +289,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p,
static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
struct nfsd_attrstat *resp)
{
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
return xdr_ressize_check(rqstp, p);
}
@@ -282,7 +297,7 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p,
static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p,
struct nfsd3_accessres *resp)
{
- p = nfs2svc_encode_fattr(rqstp, p, &resp->fh);
+ p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
*p++ = htonl(resp->access);
return xdr_ressize_check(rqstp, p);
}
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index bf6d3bc..96e5619 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -195,11 +195,9 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
}
/* Helper function for NFSv2 ACL code */
-__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
+__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat)
{
- struct kstat stat;
- fh_getattr(fhp, &stat); /* BUG */
- return encode_fattr(rqstp, p, fhp, &stat);
+ return encode_fattr(rqstp, p, fhp, stat);
}
/*
diff --git a/fs/nfsd/xdr.h b/fs/nfsd/xdr.h
index 53b1863..4f0481d 100644
--- a/fs/nfsd/xdr.h
+++ b/fs/nfsd/xdr.h
@@ -167,7 +167,7 @@ int nfssvc_encode_entry(void *, const char *name,
int nfssvc_release_fhandle(struct svc_rqst *, __be32 *, struct nfsd_fhandle *);
/* Helper functions for NFSv2 ACL code */
-__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp);
+__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp, struct kstat *stat);
__be32 *nfs2svc_decode_fh(__be32 *p, struct svc_fh *fhp);
#endif /* LINUX_NFSD_H */
diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
index 7df980e..b6d5542 100644
--- a/fs/nfsd/xdr3.h
+++ b/fs/nfsd/xdr3.h
@@ -136,6 +136,7 @@ struct nfsd3_accessres {
__be32 status;
struct svc_fh fh;
__u32 access;
+ struct kstat stat;
};
struct nfsd3_readlinkres {
@@ -225,6 +226,7 @@ struct nfsd3_getaclres {
int mask;
struct posix_acl *acl_access;
struct posix_acl *acl_default;
+ struct kstat stat;
};
/* dummy type for release */
On Fri, Feb 22, 2013 at 04:46:34PM -0500, J. Bruce Fields wrote:
> > Actually, we have precisely the same interface except for the name:
> >
> > __be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
> > vs.
> > __be32 nfsd_getattr(struct svc_fh *fh, struct kstat *stat)
> >
> > but I'm fine with your name.
> >
> > Do you want to take these patches, or should I?
*blink*
I blame being very low on caffeine; that, or being a blind idiot...
My apologies ;-/
> I guess what I'll do unless I hear otherwise is apply both your patch
> and mine to my tree for 3.9.
FWIW, I'm going to push the first part of VFS queue later tonight...
On Fri, Feb 01, 2013 at 08:15:37AM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> We're currently ignoring errors from vfs_getattr.
>
> The correct thing to do is to do the stat in the main service procedure
> not in the response encoding.
FWIW, I'd combine that with parts of commit I've got in my tree; I think
nfsd_getattr() in your variant isn't the best API here. All callers
that want nfserrno want vfsmount/dentry coming from some fhandle. Diff
below is introducing such helper and switching to its use (warning: edited
patch; only obvious editing done there, but still). It does *not* address
the issue your patch deals with (see /* BUG */ in there), but I really
think it's a better interface than your variant. FWIW, the rest of the
commit in question is switching vfs_getattr() to struct path *; I'd
edited those parts out of the diff below. Comments?
new helper: fh_getattr
A bunch of places in nfsd want vfs_getattr() done on object an fhandle
refers to, with nfs-encoded error (__be32). fh_getattr(fh, stat) does
just that; open-coded instances switched to it.
Signed-off-by: Al Viro <[email protected]>
---
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 1fc02df..4012899 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -43,7 +43,6 @@ static __be32
nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
struct nfsd3_attrstat *resp)
{
- int err;
__be32 nfserr;
dprintk("nfsd: GETATTR(3) %s\n",
@@ -55,9 +54,7 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
if (nfserr)
RETURN_STATUS(nfserr);
- err = vfs_getattr(resp->fh.fh_export->ex_path.mnt,
- resp->fh.fh_dentry, &resp->stat);
- nfserr = nfserrno(err);
+ nfserr = fh_getattr(&resp->fh, &resp->stat);
RETURN_STATUS(nfserr);
}
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 324c0ba..7af9417b 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -11,6 +11,7 @@
#include "xdr3.h"
#include "auth.h"
#include "netns.h"
+#include "vfs.h"
#define NFSDDBG_FACILITY NFSDDBG_XDR
@@ -204,10 +205,10 @@ encode_post_op_attr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
{
struct dentry *dentry = fhp->fh_dentry;
if (dentry && dentry->d_inode) {
- int err;
+ __be32 err;
struct kstat stat;
- err = vfs_getattr(fhp->fh_export->ex_path.mnt, dentry, &stat);
+ err = fh_getattr(fhp, &stat);
if (!err) {
*p++ = xdr_one; /* attributes follow */
lease_get_mtime(dentry->d_inode, &stat.mtime);
@@ -254,13 +255,12 @@ encode_wcc_data(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
*/
void fill_post_wcc(struct svc_fh *fhp)
{
- int err;
+ __be32 err;
if (fhp->fh_post_saved)
printk("nfsd: inode locked twice during operation.\n");
- err = vfs_getattr(fhp->fh_export->ex_path.mnt, fhp->fh_dentry,
- &fhp->fh_post_attr);
+ err = fh_getattr(fhp, &fhp->fh_post_attr);
fhp->fh_post_change = fhp->fh_dentry->d_inode->i_version;
if (err) {
fhp->fh_post_saved = 0;
diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index aad6d45..54c6b3d 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -26,17 +26,13 @@ static __be32
nfsd_return_attrs(__be32 err, struct nfsd_attrstat *resp)
{
if (err) return err;
- return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
- resp->fh.fh_dentry,
- &resp->stat));
+ return fh_getattr(&resp->fh, &resp->stat);
}
static __be32
nfsd_return_dirop(__be32 err, struct nfsd_diropres *resp)
{
if (err) return err;
- return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
- resp->fh.fh_dentry,
- &resp->stat));
+ return fh_getattr(&resp->fh, &resp->stat);
}
/*
* Get a file's attributes
@@ -150,9 +146,7 @@ nfsd_proc_read(struct svc_rqst *rqstp, struct nfsd_readargs *argp,
&resp->count);
if (nfserr) return nfserr;
- return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
- resp->fh.fh_dentry,
- &resp->stat));
+ return fh_getattr(&resp->fh, &resp->stat);
}
/*
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 979b421..bf6d3bc 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -4,6 +4,7 @@
* Copyright (C) 1995, 1996 Olaf Kirch <[email protected]>
*/
+#include "vfs.h"
#include "xdr.h"
#include "auth.h"
@@ -197,7 +198,7 @@ encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp,
__be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
{
struct kstat stat;
- vfs_getattr(fhp->fh_export->ex_path.mnt, fhp->fh_dentry, &stat);
+ fh_getattr(fhp, &stat); /* BUG */
return encode_fattr(rqstp, p, fhp, &stat);
}
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 359594c..5b58941 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -6,6 +6,7 @@
#define LINUX_NFSD_VFS_H
#include "nfsfh.h"
+#include "nfsd.h"
/*
* Flags for nfsd_permission
@@ -125,4 +126,11 @@ static inline void fh_drop_write(struct svc_fh *fh)
}
}
+static inline __be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
+{
+ return nfserrno(vfs_getattr(fh->fh_export->ex_path.mnt,
+ fh->fh_dentry,
+ stat));
+}
+
#endif /* LINUX_NFSD_VFS_H */