[PATCH 2.2.18pre21] Fix 3 leaks in lockd...

From: Trond Myklebust (trond.myklebust@fys.uio.no)
Date: Mon Nov 13 2000 - 14:24:59 EST


As usual, I messed up the list addresses. Here's a resend (not Cced to
Alan since he should already have received it).

Cheers,
  Trond

Forwarded message ----------------------------------

Hi Alan,

  The following patch fixes 3 leaks in the 2.2.18pre21 'lockd'
server:

  - The nlm_host h_count leaks in nlm{,4}svc_callback(). This is due
    to both nlmsvc_async_call() and the above functions incrementing
    the h_count.
    Backport the 2.4.0 definitions of nlm{clnt,svc}_async_call() in
    order to fix problem and to avoid having different APIs in 2.2.x
    and 2.4.x lockd.
    Hopefully we can clean this entire mess out in 2.5.x.

  - nlmsvc_share_file() increments the file->f_count. This causes a
    file leak when using MSDOS-style shares.
    Instead we should do as per the posix server code, and simply rely
    on the lockd file garbage collectors to not close a file with
    pending locks/shares/blocks.

  - nlm{4,}svc_proc_sm_notify() should only call nlm_release_host()
    when nlm_lookup_host() returns a non-null value.

Cheers,
  Trond

diff -u --recursive --new-file linux-2.2.18pre21/fs/lockd/clntproc.c linux-2.2.18-fix_leak/fs/lockd/clntproc.c
--- linux-2.2.18pre21/fs/lockd/clntproc.c Mon Nov 13 17:15:58 2000
+++ linux-2.2.18-fix_leak/fs/lockd/clntproc.c Mon Nov 13 19:31:55 2000
@@ -309,16 +309,14 @@
 /*
  * Generic NLM call, async version.
  */
-static int
-_nlmclnt_async_call(struct nlm_rqst *req, u32 proc, rpc_action callback,
- struct rpc_cred *cred)
+int
+nlmsvc_async_call(struct nlm_rqst *req, u32 proc, rpc_action callback)
 {
         struct nlm_host *host = req->a_host;
         struct rpc_clnt *clnt;
         struct nlm_args *argp = &req->a_args;
         struct nlm_res *resp = &req->a_res;
         struct rpc_message msg;
- int status;
 
         dprintk("lockd: call procedure %s on %s (async)\n",
                         nlm_procname(proc), host->h_name);
@@ -327,41 +325,47 @@
         if ((clnt = nlm_bind_host(host)) == NULL)
                 return -ENOLCK;
 
- /* Increment host refcount */
- nlm_get_host(host);
-
- /* bootstrap and kick off the async RPC call */
+ /* bootstrap and kick off the async RPC call */
         msg.rpc_proc = proc;
         msg.rpc_argp = argp;
         msg.rpc_resp =resp;
- msg.rpc_cred = cred;
- status = rpc_call_async(clnt, &msg, RPC_TASK_ASYNC, callback, req);
-
- if (status < 0)
- nlm_release_host(host);
- return status;
+ msg.rpc_cred = NULL;
+ return rpc_call_async(clnt, &msg, RPC_TASK_ASYNC, callback, req);
 }
 
-
 int
 nlmclnt_async_call(struct nlm_rqst *req, u32 proc, rpc_action callback)
 {
+ struct nlm_host *host = req->a_host;
+ struct rpc_clnt *clnt;
         struct nlm_args *argp = &req->a_args;
- struct file *filp = argp->lock.fl.fl_file;
- struct rpc_cred *cred = NULL;
+ struct nlm_res *resp = &req->a_res;
+ struct file *file = argp->lock.fl.fl_file;
+ struct rpc_message msg;
+ int status;
 
- if (filp)
- cred = nfs_file_cred(filp);
+ dprintk("lockd: call procedure %s on %s (async)\n",
+ nlm_procname(proc), host->h_name);
 
- return _nlmclnt_async_call(req, proc, callback, cred);
-}
+ /* If we have no RPC client yet, create one. */
+ if ((clnt = nlm_bind_host(host)) == NULL)
+ return -ENOLCK;
 
-int
-nlmsvc_async_call(struct nlm_rqst *req, u32 proc, rpc_action callback)
-{
- return _nlmclnt_async_call(req, proc, callback, NULL);
+ /* bootstrap and kick off the async RPC call */
+ msg.rpc_proc = proc;
+ msg.rpc_argp = argp;
+ msg.rpc_resp =resp;
+ if (file)
+ msg.rpc_cred = nfs_file_cred(file);
+ else
+ msg.rpc_cred = NULL;
+ /* Increment host refcount */
+ nlm_get_host(host);
+ status = rpc_call_async(clnt, &msg, RPC_TASK_ASYNC, callback, req);
+ if (status < 0)
+ nlm_release_host(host);
+ return status;
 }
-
 
 /*
  * TEST for the presence of a conflicting lock
diff -u --recursive --new-file linux-2.2.18pre21/fs/lockd/svc4proc.c linux-2.2.18-fix_leak/fs/lockd/svc4proc.c
--- linux-2.2.18pre21/fs/lockd/svc4proc.c Mon Nov 13 17:15:58 2000
+++ linux-2.2.18-fix_leak/fs/lockd/svc4proc.c Mon Nov 13 19:51:12 2000
@@ -450,8 +450,8 @@
                 if ((clnt = nlmsvc_ops->exp_getclient(&saddr)) != NULL
                  && (host = nlm_lookup_host(clnt, &saddr, 0, 0)) != NULL) {
                         nlmsvc_free_host_resources(host);
+ nlm_release_host(host);
                 }
- nlm_release_host(host);
         }
 
         return rpc_success;
@@ -472,7 +472,7 @@
         host = nlmclnt_lookup_host(&rqstp->rq_addr,
                                 rqstp->rq_prot, rqstp->rq_vers);
         if (!host) {
- rpc_free(call);
+ kfree(call);
                 return rpc_system_err;
         }
 
@@ -481,9 +481,13 @@
         memcpy(&call->a_args, resp, sizeof(*resp));
 
         if (nlmsvc_async_call(call, proc, nlm4svc_callback_exit) < 0)
- return rpc_system_err;
+ goto error;
 
         return rpc_success;
+ error:
+ kfree(call);
+ nlm_release_host(host);
+ return rpc_system_err;
 }
 
 static void
@@ -496,7 +500,7 @@
                                         task->tk_pid, -task->tk_status);
         }
         nlm_release_host(call->a_host);
- rpc_free(call);
+ kfree(call);
 }
 
 /*
diff -u --recursive --new-file linux-2.2.18pre21/fs/lockd/svclock.c linux-2.2.18-fix_leak/fs/lockd/svclock.c
--- linux-2.2.18pre21/fs/lockd/svclock.c Mon Nov 13 17:15:58 2000
+++ linux-2.2.18-fix_leak/fs/lockd/svclock.c Mon Nov 13 19:49:12 2000
@@ -531,8 +531,10 @@
         nlmsvc_insert_block(block, jiffies + 30 * HZ);
 
         /* Call the client */
- nlmsvc_async_call(&block->b_call, NLMPROC_GRANTED_MSG,
- nlmsvc_grant_callback);
+ nlm_get_host(block->b_call.a_host);
+ if (nlmsvc_async_call(&block->b_call, NLMPROC_GRANTED_MSG,
+ nlmsvc_grant_callback) < 0)
+ nlm_release_host(block->b_call.a_host);
         up(&file->f_sema);
 }
 
diff -u --recursive --new-file linux-2.2.18pre21/fs/lockd/svcproc.c linux-2.2.18-fix_leak/fs/lockd/svcproc.c
--- linux-2.2.18pre21/fs/lockd/svcproc.c Mon Nov 13 17:15:59 2000
+++ linux-2.2.18-fix_leak/fs/lockd/svcproc.c Mon Nov 13 19:51:04 2000
@@ -463,8 +463,8 @@
                 if ((clnt = nlmsvc_ops->exp_getclient(&saddr)) != NULL
                  && (host = nlm_lookup_host(clnt, &saddr, 0, 0)) != NULL) {
                         nlmsvc_free_host_resources(host);
+ nlm_release_host(host);
                 }
- nlm_release_host(host);
         }
 
         return rpc_success;
@@ -494,9 +494,13 @@
         memcpy(&call->a_args, resp, sizeof(*resp));
 
         if (nlmsvc_async_call(call, proc, nlmsvc_callback_exit) < 0)
- return rpc_system_err;
+ goto error;
 
         return rpc_success;
+ error:
+ nlm_release_host(host);
+ kfree(call);
+ return rpc_system_err;
 }
 
 static void
diff -u --recursive --new-file linux-2.2.18pre21/fs/lockd/svcshare.c linux-2.2.18-fix_leak/fs/lockd/svcshare.c
--- linux-2.2.18pre21/fs/lockd/svcshare.c Mon Nov 13 17:15:59 2000
+++ linux-2.2.18-fix_leak/fs/lockd/svcshare.c Mon Nov 13 19:07:12 2000
@@ -54,7 +54,6 @@
         share->s_owner.len = oh->len;
         share->s_next = file->f_shares;
         file->f_shares = share;
- file->f_count += 1;
 
 update:
         share->s_access = argp->fsm_access;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Nov 15 2000 - 21:00:24 EST