From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933481Ab3BLTdi (ORCPT ); Tue, 12 Feb 2013 14:33:38 -0500 Received: from mail.tpi.com ([70.99.223.143]:4904 "EHLO mail.tpi.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933153Ab3BLTdg (ORCPT ); Tue, 12 Feb 2013 14:33:36 -0500 From: Tim Gardner To: linux-kernel@vger.kernel.org Cc: Tim Gardner , Trond Myklebust , "J. Bruce Fields" , linux-nfs@vger.kernel.org Subject: [PATCH linux-next] lockd: nlmclnt_reclaim(): avoid stack overflow Date: Tue, 12 Feb 2013 12:33:15 -0700 Message-Id: <1360697595-62460-1-git-send-email-tim.gardner@canonical.com> X-Mailer: git-send-email 1.7.9.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Cc: "J. Bruce Fields" Cc: linux-nfs@vger.kernel.org Signed-off-by: Tim Gardner --- fs/lockd/clntlock.c | 8 +++++++- fs/lockd/clntproc.c | 6 ++---- include/linux/lockd/lockd.h | 3 ++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c index 4885b53..5dd23ef 100644 --- a/fs/lockd/clntlock.c +++ b/fs/lockd/clntlock.c @@ -220,10 +220,15 @@ 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) + return -ENOMEM; + allow_signal(SIGKILL); down_write(&host->h_rwsem); @@ -253,7 +258,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 +284,6 @@ restart: /* Release host handle after use */ nlmclnt_release_host(host); lockd_down(net); + kfree(req); return 0; } diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c index 54f9e6c..b43114c 100644 --- a/fs/lockd/clntproc.c +++ b/fs/lockd/clntproc.c @@ -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); diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h index f5a051a..a395f1e 100644 --- a/include/linux/lockd/lockd.h +++ b/include/linux/lockd/lockd.h @@ -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 *); /* -- 1.7.9.5