NFSD: Remove a layering violation when encoding lock_denied
authorChuck Lever <chuck.lever@oracle.com>
Thu, 12 Oct 2023 17:46:39 +0000 (13:46 -0400)
committerChuck Lever <chuck.lever@oracle.com>
Mon, 16 Oct 2023 16:44:30 +0000 (12:44 -0400)
An XDR encoder is responsible for marshaling results, not releasing
memory that was allocated by the upper layer. We have .op_release
for that purpose.

Move the release of the ld_owner.data string to op_release functions
for LOCK and LOCKT.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
fs/nfsd/nfs4proc.c
fs/nfsd/nfs4state.c
fs/nfsd/nfs4xdr.c
fs/nfsd/xdr4.h

index 60262fd27b15147284936734322e5da275c3a957..f288039545e3f3d43bd25f0b06ce0b7517ddc7cc 100644 (file)
@@ -3210,6 +3210,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
        },
        [OP_LOCK] = {
                .op_func = nfsd4_lock,
+               .op_release = nfsd4_lock_release,
                .op_flags = OP_MODIFIES_SOMETHING |
                                OP_NONTRIVIAL_ERROR_ENCODE,
                .op_name = "OP_LOCK",
@@ -3218,6 +3219,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
        },
        [OP_LOCKT] = {
                .op_func = nfsd4_lockt,
+               .op_release = nfsd4_lockt_release,
                .op_flags = OP_NONTRIVIAL_ERROR_ENCODE,
                .op_name = "OP_LOCKT",
                .op_rsize_bop = nfsd4_lock_rsize,
index 07840ee721ef734f21895b267cb2c0ca4db724f1..305c353a416ca96c46001c2573b06441324186f0 100644 (file)
@@ -7773,6 +7773,14 @@ out:
        return status;
 }
 
+void nfsd4_lock_release(union nfsd4_op_u *u)
+{
+       struct nfsd4_lock *lock = &u->lock;
+       struct nfsd4_lock_denied *deny = &lock->lk_denied;
+
+       kfree(deny->ld_owner.data);
+}
+
 /*
  * The NFSv4 spec allows a client to do a LOCKT without holding an OPEN,
  * so we do a temporary open here just to get an open file to pass to
@@ -7878,6 +7886,14 @@ out:
        return status;
 }
 
+void nfsd4_lockt_release(union nfsd4_op_u *u)
+{
+       struct nfsd4_lockt *lockt = &u->lockt;
+       struct nfsd4_lock_denied *deny = &lockt->lt_denied;
+
+       kfree(deny->ld_owner.data);
+}
+
 __be32
 nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
            union nfsd4_op_u *u)
index dafb3a91235ec2cf73d25ed7d7abe98076c003a5..26e7bb6d32ab29b1bebf08fcb259888bfb96b21e 100644 (file)
@@ -3990,28 +3990,16 @@ nfsd4_encode_lock_denied(struct xdr_stream *xdr, struct nfsd4_lock_denied *ld)
        struct xdr_netobj *conf = &ld->ld_owner;
        __be32 *p;
 
-again:
        p = xdr_reserve_space(xdr, 32 + XDR_LEN(conf->len));
-       if (!p) {
-               /*
-                * Don't fail to return the result just because we can't
-                * return the conflicting open:
-                */
-               if (conf->len) {
-                       kfree(conf->data);
-                       conf->len = 0;
-                       conf->data = NULL;
-                       goto again;
-               }
+       if (!p)
                return nfserr_resource;
-       }
+
        p = xdr_encode_hyper(p, ld->ld_start);
        p = xdr_encode_hyper(p, ld->ld_length);
        *p++ = cpu_to_be32(ld->ld_type);
        if (conf->len) {
                p = xdr_encode_opaque_fixed(p, &ld->ld_clientid, 8);
                p = xdr_encode_opaque(p, conf->data, conf->len);
-               kfree(conf->data);
        }  else {  /* non - nfsv4 lock in conflict, no clientid nor owner */
                p = xdr_encode_hyper(p, (u64)0); /* clientid */
                *p++ = cpu_to_be32(0); /* length of owner name */
index aba07d5378fc26a4504b7f49a621b1bdf4be92b4..e6c9daae196e246dfc80eaafc32ef60576299662 100644 (file)
@@ -292,12 +292,8 @@ struct nfsd4_lock {
        } v;
 
        /* response */
-       union {
-               struct {
-                       stateid_t               stateid;
-               } ok;
-               struct nfsd4_lock_denied        denied;
-       } u;
+       stateid_t                       lk_resp_stateid;
+       struct nfsd4_lock_denied        lk_denied;
 };
 #define lk_new_open_seqid       v.new.open_seqid
 #define lk_new_open_stateid     v.new.open_stateid
@@ -307,20 +303,15 @@ struct nfsd4_lock {
 #define lk_old_lock_stateid     v.old.lock_stateid
 #define lk_old_lock_seqid       v.old.lock_seqid
 
-#define lk_resp_stateid u.ok.stateid
-#define lk_denied       u.denied
-
-
 struct nfsd4_lockt {
        u32                             lt_type;
        clientid_t                      lt_clientid;
        struct xdr_netobj               lt_owner;
        u64                             lt_offset;
        u64                             lt_length;
-       struct nfsd4_lock_denied        lt_denied;
+       struct nfsd4_lock_denied        lt_denied;
 };
 
 struct nfsd4_locku {
        u32             lu_type;
        u32             lu_seqid;
@@ -942,8 +933,10 @@ extern __be32 nfsd4_open_downgrade(struct svc_rqst *rqstp,
                struct nfsd4_compound_state *, union nfsd4_op_u *u);
 extern __be32 nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *,
                union nfsd4_op_u *u);
+extern void nfsd4_lock_release(union nfsd4_op_u *u);
 extern __be32 nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *,
                union nfsd4_op_u *u);
+extern void nfsd4_lockt_release(union nfsd4_op_u *u);
 extern __be32 nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *,
                union nfsd4_op_u *u);
 extern __be32