On Fri, 2009-06-12 at 17:07 -0400, Chuck Lever wrote:
> On Jun 12, 2009, at 4:55 PM, Trond Myklebust wrote:
>
> > On Fri, 2009-05-01 at 12:36 -0400, Chuck Lever wrote:
> >> Introduce data structures and xdr_stream-based decoding functions for
> >> unmarshalling mountd status codes properly.
> >>
> >> Mountd version 3 uses specific standard error return codes that are
> >> not errno values and not NFS3ERR_ values. These have a well-defined
> >> standard mapping to local errno values. Introduce data structures
> >> and a decoder function that map these status codes to local errno
> >> values properly. This is new functionality (but not used yet).
> >>
> >> Version 1 mountd status values are defined by RFC 1094 as UNIX error
> >> values (errno values). Errno values on heterogeneous systems do not
> >> necessarily match each other. To avoid exposing possibly incorrect
> >> errno values to upper layers, the current XDR decoder converts all
> >> non-zero MNT version 1 status codes to -EACCES.
> >>
> >> The OpenGroup XNFS standard provides a mapping similar to but smaller
> >> than the version 3 error codes. Implement a decoder that uses the
> >> XNFS
> >> error codes, replacing the current decoder.
> >>
> >> For both mountd protocol versions, map unrecognized errors to -
> >> EACCES.
> >>
> >> Finally we introduce a replacement data structure for mnt_fhstatus
> >> at this time, which is used by the new XDR decoders. In addition to
> >> documenting that the status value returned by the XDR decoders is
> >> always an errno, this new structure will be expanded in subsequent
> >> patches.
> >>
> >> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> >> ---
> >>
> >> fs/nfs/mount_clnt.c | 134 +++++++++++++++++++++++++++++++++++++++++
> >> ++++++++++
> >> 1 files changed, 134 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> >> index 7b81914..cdda067 100644
> >> --- a/fs/nfs/mount_clnt.c
> >> +++ b/fs/nfs/mount_clnt.c
> >> @@ -29,6 +29,8 @@
> >> * XDR data type sizes
> >> */
> >> #define encode_dirpath_sz XDR_QUADLEN(sizeof(u32) + MNTPATHLEN)
> >> +#define MNT_status_sz XDR_QUADLEN(sizeof(u32))
> >> +#define MNT_fhs_status_sz XDR_QUADLEN(sizeof(u32))
> >
> > XDR_QUADLEN() applies to strings only. Please just use naked numbers
> > here. I've already fixed up the previous patch that did this.
>
> OK, I'll fix these up. So the redriven patch applies, did you change
> encode_dirpath_sz to be:
>
> #define encode_dirpath_sz (1 + XDR_QUADLEN(MNTPATHLEN))
Yep. That's precisely what I did. See attachment.
> Actually, just send what you have so I can see what exactly you want
> here.
>
> But, where does it say XDR_QUADLEN applies to strings only? The
> documenting comment just says "Buffer adjustment".
We should probably fix the comment then. The point of XDR_QUADLEN() is
to pad an array so that it is correctly aligned, and then convert it
into a count of words.
> >> /*
> >> * XDR argument and result sizes
> >> @@ -61,6 +63,83 @@ enum {
> >>
> >> static struct rpc_program mnt_program;
> >>
> >> +/*
> >> + * Defined by OpenGroup XNFS Version 3W, chapter 8
> >> + */
> >> +enum mountstat {
> >> + MNT_OK = 0,
> >> + MNT_EPERM = 1,
> >> + MNT_ENOENT = 2,
> >> + MNT_EACCES = 13,
> >> + MNT_EINVAL = 22,
> >> +};
> >> +
> >> +#define MNT_MAPERRNO(name) \
> >> + { \
> >> + .status = MNT_E##name, \
> >> + .errno = -E##name, \
> >> + }
> >
> > It would be nice to avoid this kind of macro. Please inline instead.
>
> Would you like the PROC macro replaced with open code as well?
Yes please. I know the PROC macro has a longer history, but it is tough
to read if you're a newbie nfs developer...
> >> +
> >> +static struct {
> >> + u32 status;
> >> + int errno;
> >> +} mnt_errtbl[] = {
> >> + {
> >> + .status = MNT_OK,
> >> + .errno = 0,
> >> + },
> >> + MNT_MAPERRNO(PERM),
> >> + MNT_MAPERRNO(NOENT),
> >> + MNT_MAPERRNO(ACCES),
> >> + MNT_MAPERRNO(INVAL),
> >> +};
> >> +
> >> +/*
> >> + * Defined by RFC 1813, section 5.1.5
> >> + */
> >> +enum mountstat3 {
> >> + MNT3_OK = 0, /* no error */
> >> + MNT3ERR_PERM = 1, /* Not owner */
> >> + MNT3ERR_NOENT = 2, /* No such file or directory */
> >> + MNT3ERR_IO = 5, /* I/O error */
> >> + MNT3ERR_ACCES = 13, /* Permission denied */
> >> + MNT3ERR_NOTDIR = 20, /* Not a directory */
> >> + MNT3ERR_INVAL = 22, /* Invalid argument */
> >> + MNT3ERR_NAMETOOLONG = 63, /* Filename too long */
> >> + MNT3ERR_NOTSUPP = 10004, /* Operation not supported */
> >> + MNT3ERR_SERVERFAULT = 10006, /* A failure on the server */
> >> +};
> >> +
> >> +#define MNT3_MAPERRNO(name) \
> >> + { \
> >> + .status = MNT3ERR_##name, \
> >> + .errno = -E##name, \
> >> + }
> >
> > See above
> >
> >> +static struct {
> >> + u32 status;
> >> + int errno;
> >> +} mnt3_errtbl[] = {
> >> + {
> >> + .status = MNT3_OK,
> >> + .errno = 0,
> >> + },
> >> + MNT3_MAPERRNO(PERM),
> >> + MNT3_MAPERRNO(NOENT),
> >> + MNT3_MAPERRNO(IO),
> >> + MNT3_MAPERRNO(ACCES),
> >> + MNT3_MAPERRNO(NOTDIR),
> >> + MNT3_MAPERRNO(INVAL),
> >> + MNT3_MAPERRNO(NAMETOOLONG),
> >> + MNT3_MAPERRNO(NOTSUPP),
> >> + MNT3_MAPERRNO(SERVERFAULT),
> >> +};
> >> +
> >> +struct mountres {
> >> + int errno;
> >> + struct nfs_fh *fh;
> >> +};
> >> +
> >> struct mnt_fhstatus {
> >> u32 status;
> >> struct nfs_fh *fh;
> >> @@ -179,6 +258,61 @@ static int xdr_decode_fhstatus(struct rpc_rqst
> >> *req, __be32 *p,
> >> return 0;
> >> }
> >>
> >> +/*
> >> + * RFC 1094: "A non-zero status indicates some sort of error. In
> >> this
> >> + * case, the status is a UNIX error number." This can be
> >> problematic
> >> + * if the server and client use different errno values for the same
> >> + * error.
> >> + *
> >> + * However, the OpenGroup XNFS spec provides a simple mapping that
> >> is
> >> + * independent of local errno values on the server and the client.
> >> + */
> >> +static int decode_status(struct xdr_stream *xdr, struct mountres
> >> *res)
> >> +{
> >> + unsigned int i;
> >> + u32 status;
> >> + __be32 *p;
> >> +
> >> + p = xdr_inline_decode(xdr, sizeof(status));
> >> + if (unlikely(p == NULL))
> >> + return -EIO;
> >> + status = ntohl(*p);
> >> +
> >> + for (i = 0; i <= ARRAY_SIZE(mnt_errtbl); i++) {
> >> + if (mnt_errtbl[i].status == status) {
> >> + res->errno = mnt_errtbl[i].errno;
> >> + return 0;
> >> + }
> >> + }
> >> +
> >> + dprintk("NFS: unrecognized MNT status code: %u\n", status);
> >> + res->errno = -EACCES;
> >> + return 0;
> >> +}
> >> +
> >> +static int decode_fhs_status(struct xdr_stream *xdr, struct
> >> mountres *res)
> >> +{
> >> + unsigned int i;
> >> + u32 status;
> >> + __be32 *p;
> >> +
> >> + p = xdr_inline_decode(xdr, sizeof(status));
> >> + if (unlikely(p == NULL))
> >> + return -EIO;
> >> + status = ntohl(*p);
> >> +
> >> + for (i = 0; i <= ARRAY_SIZE(mnt3_errtbl); i++) {
> >> + if (mnt3_errtbl[i].status == status) {
> >> + res->errno = mnt3_errtbl[i].errno;
> >> + return 0;
> >> + }
> >> + }
> >> +
> >> + dprintk("NFS: unrecognized MNT3 status code: %u\n", status);
> >> + res->errno = -EACCES;
> >> + return 0;
> >> +}
> >> +
> >> static int xdr_decode_fhstatus3(struct rpc_rqst *req, __be32 *p,
> >> struct mnt_fhstatus *res)
> >> {
> >>
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer
> >
> > NetApp
> > Trond.Myklebust@xxxxxxxxxx
> > www.netapp.com
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--- Begin Message ---
- Subject: No Subject
- From: Chuck Lever <chuck.lever@xxxxxxxxxx>
- Date:
- Nfs: Use xdr_stream-based XDR encoder for MNT's dirpath argument
Check the length of the supplied dirpath, and see that it fits
properly in the RPC buffer.
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
[Trond.Myklebust@xxxxxxxxxx: fixed up definition of encode_dirpath_sz]
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
---
fs/nfs/mount_clnt.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
index af45a37..93361af 100644
--- a/fs/nfs/mount_clnt.c
+++ b/fs/nfs/mount_clnt.c
@@ -21,6 +21,21 @@
#endif
/*
+ * Defined by RFC 1094, section A.3; and RFC 1813, section 5.1.4
+ */
+#define MNTPATHLEN (1024)
+
+/*
+ * XDR data type sizes
+ */
+#define encode_dirpath_sz (1 + XDR_QUADLEN(MNTPATHLEN))
+
+/*
+ * XDR argument and result sizes
+ */
+#define MNT_enc_dirpath_sz encode_dirpath_sz
+
+/*
* Defined by RFC 1094, section A.5
*/
enum {
@@ -135,6 +150,31 @@ static int xdr_encode_dirpath(struct rpc_rqst *req, __be32 *p,
return 0;
}
+static int encode_mntdirpath(struct xdr_stream *xdr, const char *pathname)
+{
+ const u32 pathname_len = strlen(pathname);
+ __be32 *p;
+
+ if (unlikely(pathname_len > MNTPATHLEN))
+ return -EIO;
+
+ p = xdr_reserve_space(xdr, sizeof(u32) + pathname_len);
+ if (unlikely(p == NULL))
+ return -EIO;
+ xdr_encode_opaque(p, pathname, pathname_len);
+
+ return 0;
+}
+
+static int mnt_enc_dirpath(struct rpc_rqst *req, __be32 *p,
+ const char *dirpath)
+{
+ struct xdr_stream xdr;
+
+ xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+ return encode_mntdirpath(&xdr, dirpath);
+}
+
static int xdr_decode_fhstatus(struct rpc_rqst *req, __be32 *p,
struct mnt_fhstatus *res)
{
@@ -164,16 +204,15 @@ static int xdr_decode_fhstatus3(struct rpc_rqst *req, __be32 *p,
return 0;
}
-#define MNT_dirpath_sz (1 + 256)
#define MNT_fhstatus_sz (1 + 8)
#define MNT_fhstatus3_sz (1 + 16)
static struct rpc_procinfo mnt_procedures[] = {
[MOUNTPROC_MNT] = {
.p_proc = MOUNTPROC_MNT,
- .p_encode = (kxdrproc_t) xdr_encode_dirpath,
+ .p_encode = (kxdrproc_t)mnt_enc_dirpath,
.p_decode = (kxdrproc_t) xdr_decode_fhstatus,
- .p_arglen = MNT_dirpath_sz,
+ .p_arglen = MNT_enc_dirpath_sz,
.p_replen = MNT_fhstatus_sz,
.p_statidx = MOUNTPROC_MNT,
.p_name = "MOUNT",
@@ -183,9 +222,9 @@ static struct rpc_procinfo mnt_procedures[] = {
static struct rpc_procinfo mnt3_procedures[] = {
[MOUNTPROC3_MNT] = {
.p_proc = MOUNTPROC3_MNT,
- .p_encode = (kxdrproc_t) xdr_encode_dirpath,
+ .p_encode = (kxdrproc_t)mnt_enc_dirpath,
.p_decode = (kxdrproc_t) xdr_decode_fhstatus3,
- .p_arglen = MNT_dirpath_sz,
+ .p_arglen = MNT_enc_dirpath_sz,
.p_replen = MNT_fhstatus3_sz,
.p_statidx = MOUNTPROC3_MNT,
.p_name = "MOUNT",
--- End Message ---
