NFSD: Fix nfs4_verifier memory alignment
Chuck Lever [Fri, 2 Mar 2012 22:13:50 +0000 (17:13 -0500)]
Clean up due to code review.

The nfs4_verifier's data field is not guaranteed to be u32-aligned.
Casting an array of chars to a u32 * is considered generally
hazardous.

We can fix most of this by using a __be32 array to generate the
verifier's contents and then byte-copying it into the verifier field.

However, there is one spot where there is a backwards compatibility
constraint: the do_nfsd_create() call expects a verifier which is
32-bit aligned.  Fix this spot by forcing the alignment of the create
verifier in the nfsd4_open args structure.

Also, sizeof(nfs4_verifer) is the size of the in-core verifier data
structure, but NFS4_VERIFIER_SIZE is the number of octets in an XDR'd
verifier.  The two are not interchangeable, even if they happen to
have the same value.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

fs/nfsd/nfs4proc.c
fs/nfsd/nfs4state.c
fs/nfsd/nfs4xdr.c
fs/nfsd/xdr4.h

index 3f7dbc4..2a90366 100644 (file)
@@ -489,14 +489,20 @@ nfsd4_access(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                           &access->ac_supported);
 }
 
+static void gen_boot_verifier(nfs4_verifier *verifier)
+{
+       __be32 verf[2];
+
+       verf[0] = (__be32)nfssvc_boot.tv_sec;
+       verf[1] = (__be32)nfssvc_boot.tv_usec;
+       memcpy(verifier->data, verf, sizeof(verifier->data));
+}
+
 static __be32
 nfsd4_commit(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
             struct nfsd4_commit *commit)
 {
-       u32 *p = (u32 *)commit->co_verf.data;
-       *p++ = nfssvc_boot.tv_sec;
-       *p++ = nfssvc_boot.tv_usec;
-
+       gen_boot_verifier(&commit->co_verf);
        return nfsd_commit(rqstp, &cstate->current_fh, commit->co_offset,
                             commit->co_count);
 }
@@ -873,7 +879,6 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 {
        stateid_t *stateid = &write->wr_stateid;
        struct file *filp = NULL;
-       u32 *p;
        __be32 status = nfs_ok;
        unsigned long cnt;
 
@@ -895,9 +900,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
        cnt = write->wr_buflen;
        write->wr_how_written = write->wr_stable_how;
-       p = (u32 *)write->wr_verifier.data;
-       *p++ = nfssvc_boot.tv_sec;
-       *p++ = nfssvc_boot.tv_usec;
+       gen_boot_verifier(&write->wr_verifier);
 
        status =  nfsd_write(rqstp, &cstate->current_fh, filp,
                             write->wr_offset, rqstp->rq_vec, write->wr_vlen,
index cdc406a..e318964 100644 (file)
@@ -1174,12 +1174,12 @@ static void gen_clid(struct nfs4_client *clp)
 
 static void gen_confirm(struct nfs4_client *clp)
 {
+       __be32 verf[2];
        static u32 i;
-       u32 *p;
 
-       p = (u32 *)clp->cl_confirm.data;
-       *p++ = get_seconds();
-       *p++ = i++;
+       verf[0] = (__be32)get_seconds();
+       verf[1] = (__be32)i++;
+       memcpy(clp->cl_confirm.data, verf, sizeof(clp->cl_confirm.data));
 }
 
 static struct nfs4_stid *find_stateid(struct nfs4_client *cl, stateid_t *t)
index f8fcddc..bcd8904 100644 (file)
@@ -749,14 +749,14 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
                                goto out;
                        break;
                case NFS4_CREATE_EXCLUSIVE:
-                       READ_BUF(8);
-                       COPYMEM(open->op_verf.data, 8);
+                       READ_BUF(NFS4_VERIFIER_SIZE);
+                       COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
                        break;
                case NFS4_CREATE_EXCLUSIVE4_1:
                        if (argp->minorversion < 1)
                                goto xdr_error;
-                       READ_BUF(8);
-                       COPYMEM(open->op_verf.data, 8);
+                       READ_BUF(NFS4_VERIFIER_SIZE);
+                       COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
                        status = nfsd4_decode_fattr(argp, open->op_bmval,
                                &open->op_iattr, &open->op_acl);
                        if (status)
@@ -989,8 +989,8 @@ nfsd4_decode_setclientid(struct nfsd4_compoundargs *argp, struct nfsd4_setclient
 {
        DECODE_HEAD;
 
-       READ_BUF(8);
-       COPYMEM(setclientid->se_verf.data, 8);
+       READ_BUF(NFS4_VERIFIER_SIZE);
+       COPYMEM(setclientid->se_verf.data, NFS4_VERIFIER_SIZE);
 
        status = nfsd4_decode_opaque(argp, &setclientid->se_name);
        if (status)
@@ -1015,9 +1015,9 @@ nfsd4_decode_setclientid_confirm(struct nfsd4_compoundargs *argp, struct nfsd4_s
 {
        DECODE_HEAD;
 
-       READ_BUF(8 + sizeof(nfs4_verifier));
+       READ_BUF(8 + NFS4_VERIFIER_SIZE);
        COPYMEM(&scd_c->sc_clientid, 8);
-       COPYMEM(&scd_c->sc_confirm, sizeof(nfs4_verifier));
+       COPYMEM(&scd_c->sc_confirm, NFS4_VERIFIER_SIZE);
 
        DECODE_TAIL;
 }
@@ -2659,8 +2659,8 @@ nfsd4_encode_commit(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
        __be32 *p;
 
        if (!nfserr) {
-               RESERVE_SPACE(8);
-               WRITEMEM(commit->co_verf.data, 8);
+               RESERVE_SPACE(NFS4_VERIFIER_SIZE);
+               WRITEMEM(commit->co_verf.data, NFS4_VERIFIER_SIZE);
                ADJUST_ARGS();
        }
        return nfserr;
@@ -3020,7 +3020,7 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
        if (resp->xbuf->page_len)
                return nfserr_resource;
 
-       RESERVE_SPACE(8);  /* verifier */
+       RESERVE_SPACE(NFS4_VERIFIER_SIZE);
        savep = p;
 
        /* XXX: Following NFSv3, we ignore the READDIR verifier for now. */
@@ -3221,9 +3221,9 @@ nfsd4_encode_setclientid(struct nfsd4_compoundres *resp, __be32 nfserr, struct n
        __be32 *p;
 
        if (!nfserr) {
-               RESERVE_SPACE(8 + sizeof(nfs4_verifier));
+               RESERVE_SPACE(8 + NFS4_VERIFIER_SIZE);
                WRITEMEM(&scd->se_clientid, 8);
-               WRITEMEM(&scd->se_confirm, sizeof(nfs4_verifier));
+               WRITEMEM(&scd->se_confirm, NFS4_VERIFIER_SIZE);
                ADJUST_ARGS();
        }
        else if (nfserr == nfserr_clid_inuse) {
@@ -3244,7 +3244,7 @@ nfsd4_encode_write(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_w
                RESERVE_SPACE(16);
                WRITE32(write->wr_bytes_written);
                WRITE32(write->wr_how_written);
-               WRITEMEM(write->wr_verifier.data, 8);
+               WRITEMEM(write->wr_verifier.data, NFS4_VERIFIER_SIZE);
                ADJUST_ARGS();
        }
        return nfserr;
index b89781f..1b35015 100644 (file)
@@ -228,7 +228,8 @@ struct nfsd4_open {
        u32             op_createmode;      /* request */
        u32             op_bmval[3];        /* request */
        struct iattr    iattr;              /* UNCHECKED4, GUARDED4, EXCLUSIVE4_1 */
-       nfs4_verifier   verf;               /* EXCLUSIVE4 */
+       nfs4_verifier   op_verf __attribute__((aligned(32)));
+                                           /* EXCLUSIVE4 */
        clientid_t      op_clientid;        /* request */
        struct xdr_netobj op_owner;           /* request */
        u32             op_seqid;           /* request */
@@ -247,7 +248,6 @@ struct nfsd4_open {
        struct nfs4_acl *op_acl;
 };
 #define op_iattr       iattr
-#define op_verf                verf
 
 struct nfsd4_open_confirm {
        stateid_t       oc_req_stateid          /* request */;