lockd: nlmclnt_reclaim(): avoid stack overflow
authorTim Gardner <tim.gardner@canonical.com>
Wed, 13 Feb 2013 15:40:16 +0000 (08:40 -0700)
committerJ. Bruce Fields <bfields@redhat.com>
Fri, 15 Feb 2013 16:29:38 +0000 (11:29 -0500)
Even though nlmclnt_reclaim() is only one call into the stack frame,
928 bytes on the stack seems like a lot. Recode to dynamically
allocate the request structure once from within the reclaimer task,
then pass this pointer into nlmclnt_reclaim() for reuse on
subsequent calls.

smatch analysis:

fs/lockd/clntproc.c:620 nlmclnt_reclaim() warn: 'reqst' puts
 928 bytes on stack

Also remove redundant assignment of 0 after memset.

Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
fs/lockd/clntlock.c
fs/lockd/clntproc.c
include/linux/lockd/lockd.h

index 4885b53e56bd8cc65fc96d5f7f7a1e531c6ffb40..6cd673d34fb9abd6bcede38b4ba390bb9b920e41 100644 (file)
@@ -220,10 +220,19 @@ reclaimer(void *ptr)
 {
        struct nlm_host   *host = (struct nlm_host *) ptr;
        struct nlm_wait   *block;
+       struct nlm_rqst   *req;
        struct file_lock *fl, *next;
        u32 nsmstate;
        struct net *net = host->net;
 
+       req = kmalloc(sizeof(*req), GFP_KERNEL);
+       if (!req) {
+               printk(KERN_ERR "lockd: reclaimer unable to alloc memory."
+                               " Locks for %s won't be reclaimed!\n",
+                               host->h_name);
+               return 0;
+       }
+
        allow_signal(SIGKILL);
 
        down_write(&host->h_rwsem);
@@ -253,7 +262,7 @@ restart:
                 */
                if (signalled())
                        continue;
-               if (nlmclnt_reclaim(host, fl) != 0)
+               if (nlmclnt_reclaim(host, fl, req) != 0)
                        continue;
                list_add_tail(&fl->fl_u.nfs_fl.list, &host->h_granted);
                if (host->h_nsmstate != nsmstate) {
@@ -279,5 +288,6 @@ restart:
        /* Release host handle after use */
        nlmclnt_release_host(host);
        lockd_down(net);
+       kfree(req);
        return 0;
 }
index 54f9e6ce0430ae88d742709bc47499c0d1b71731..b43114c4332addda4712c278b32bbe39980ab1da 100644 (file)
@@ -615,17 +615,15 @@ out_unlock:
  * RECLAIM: Try to reclaim a lock
  */
 int
-nlmclnt_reclaim(struct nlm_host *host, struct file_lock *fl)
+nlmclnt_reclaim(struct nlm_host *host, struct file_lock *fl,
+               struct nlm_rqst *req)
 {
-       struct nlm_rqst reqst, *req;
        int             status;
 
-       req = &reqst;
        memset(req, 0, sizeof(*req));
        locks_init_lock(&req->a_args.lock.fl);
        locks_init_lock(&req->a_res.lock.fl);
        req->a_host  = host;
-       req->a_flags = 0;
 
        /* Set up the argument struct */
        nlmclnt_setlockargs(req, fl);
index f5a051a79273c7f3fcdb1a18f4c9e2522e68cc85..a395f1e7998f41539a8681980d0afdffe7332e43 100644 (file)
@@ -212,7 +212,8 @@ int           nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout)
 __be32           nlmclnt_grant(const struct sockaddr *addr,
                                const struct nlm_lock *lock);
 void             nlmclnt_recovery(struct nlm_host *);
-int              nlmclnt_reclaim(struct nlm_host *, struct file_lock *);
+int              nlmclnt_reclaim(struct nlm_host *, struct file_lock *,
+                                 struct nlm_rqst *);
 void             nlmclnt_next_cookie(struct nlm_cookie *);
 
 /*