All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org,
	hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
	cluster-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH 02/10] nfsd: fix potential lease memory leak in nfs4_setlease
Date: Sat, 23 Aug 2014 10:41:10 -0400	[thread overview]
Message-ID: <1408804878-1331-3-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1408804878-1331-1-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>

It's unlikely to ever occur, but if there were already a lease set on
the file then we could end up getting back a different pointer on a
successful setlease attempt than the one we allocated. If that happens,
the one we allocated could leak.

In practice, I don't think this will happen due to the fact that we only
try to set up the lease once per nfs4_file, but this error handling is a
bit more correct given the current lease API.

Cc: J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
---
 fs/nfsd/nfs4state.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fd5ff4b17292..29fac18d9102 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3774,7 +3774,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
 static int nfs4_setlease(struct nfs4_delegation *dp)
 {
 	struct nfs4_file *fp = dp->dl_stid.sc_file;
-	struct file_lock *fl;
+	struct file_lock *fl, *ret;
 	struct file *filp;
 	int status = 0;
 
@@ -3788,11 +3788,14 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
 		return -EBADF;
 	}
 	fl->fl_file = filp;
-	status = vfs_setlease(filp, fl->fl_type, &fl);
+	ret = fl;
+	status = vfs_setlease(filp, fl->fl_type, &ret);
 	if (status) {
 		locks_free_lock(fl);
 		goto out_fput;
 	}
+	if (ret != fl)
+		locks_free_lock(fl);
 	spin_lock(&state_lock);
 	spin_lock(&fp->fi_lock);
 	/* Did the lease get broken before we took the lock? */
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@primarydata.com>
To: linux-fsdevel@vger.kernel.org
Cc: bfields@fieldses.org, hch@infradead.org,
	cluster-devel@redhat.com, linux-cifs@vger.kernel.org,
	linux-nfs@vger.kernel.org
Subject: [PATCH 02/10] nfsd: fix potential lease memory leak in nfs4_setlease
Date: Sat, 23 Aug 2014 10:41:10 -0400	[thread overview]
Message-ID: <1408804878-1331-3-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1408804878-1331-1-git-send-email-jlayton@primarydata.com>

It's unlikely to ever occur, but if there were already a lease set on
the file then we could end up getting back a different pointer on a
successful setlease attempt than the one we allocated. If that happens,
the one we allocated could leak.

In practice, I don't think this will happen due to the fact that we only
try to set up the lease once per nfs4_file, but this error handling is a
bit more correct given the current lease API.

Cc: J. Bruce Fields <bfields@fieldses.org>
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/nfsd/nfs4state.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fd5ff4b17292..29fac18d9102 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3774,7 +3774,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
 static int nfs4_setlease(struct nfs4_delegation *dp)
 {
 	struct nfs4_file *fp = dp->dl_stid.sc_file;
-	struct file_lock *fl;
+	struct file_lock *fl, *ret;
 	struct file *filp;
 	int status = 0;
 
@@ -3788,11 +3788,14 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
 		return -EBADF;
 	}
 	fl->fl_file = filp;
-	status = vfs_setlease(filp, fl->fl_type, &fl);
+	ret = fl;
+	status = vfs_setlease(filp, fl->fl_type, &ret);
 	if (status) {
 		locks_free_lock(fl);
 		goto out_fput;
 	}
+	if (ret != fl)
+		locks_free_lock(fl);
 	spin_lock(&state_lock);
 	spin_lock(&fp->fi_lock);
 	/* Did the lease get broken before we took the lock? */
-- 
1.9.3


WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@primarydata.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 02/10] nfsd: fix potential lease memory leak in nfs4_setlease
Date: Sat, 23 Aug 2014 10:41:10 -0400	[thread overview]
Message-ID: <1408804878-1331-3-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1408804878-1331-1-git-send-email-jlayton@primarydata.com>

It's unlikely to ever occur, but if there were already a lease set on
the file then we could end up getting back a different pointer on a
successful setlease attempt than the one we allocated. If that happens,
the one we allocated could leak.

In practice, I don't think this will happen due to the fact that we only
try to set up the lease once per nfs4_file, but this error handling is a
bit more correct given the current lease API.

Cc: J. Bruce Fields <bfields@fieldses.org>
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/nfsd/nfs4state.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fd5ff4b17292..29fac18d9102 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3774,7 +3774,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
 static int nfs4_setlease(struct nfs4_delegation *dp)
 {
 	struct nfs4_file *fp = dp->dl_stid.sc_file;
-	struct file_lock *fl;
+	struct file_lock *fl, *ret;
 	struct file *filp;
 	int status = 0;
 
@@ -3788,11 +3788,14 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
 		return -EBADF;
 	}
 	fl->fl_file = filp;
-	status = vfs_setlease(filp, fl->fl_type, &fl);
+	ret = fl;
+	status = vfs_setlease(filp, fl->fl_type, &ret);
 	if (status) {
 		locks_free_lock(fl);
 		goto out_fput;
 	}
+	if (ret != fl)
+		locks_free_lock(fl);
 	spin_lock(&state_lock);
 	spin_lock(&fp->fi_lock);
 	/* Did the lease get broken before we took the lock? */
-- 
1.9.3



  parent reply	other threads:[~2014-08-23 14:41 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-23 14:41 [PATCH 00/10] locks/nfsd: internal lease API overhaul Jeff Layton
2014-08-23 14:41 ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41 ` Jeff Layton
2014-08-23 14:41 ` [PATCH 04/10] locks: clean up vfs_setlease kerneldoc comments Jeff Layton
2014-08-23 14:41   ` [Cluster-devel] " Jeff Layton
2014-08-24 15:51   ` Christoph Hellwig
2014-08-24 15:51     ` [Cluster-devel] " Christoph Hellwig
     [not found]     ` <20140824155117.GC15908-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-25 20:11       ` J. Bruce Fields
2014-08-25 20:11         ` [Cluster-devel] " J. Bruce Fields
2014-08-25 20:11         ` J. Bruce Fields
2014-08-23 14:41 ` [PATCH 07/10] locks: define a lm_setup handler for leases Jeff Layton
2014-08-23 14:41   ` [Cluster-devel] " Jeff Layton
     [not found]   ` <1408804878-1331-8-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 15:58     ` Christoph Hellwig
2014-08-24 15:58       ` [Cluster-devel] " Christoph Hellwig
2014-08-24 15:58       ` Christoph Hellwig
2014-08-25  1:19       ` Jeff Layton
2014-08-25  1:19         ` [Cluster-devel] " Jeff Layton
2014-08-26 13:58         ` Christoph Hellwig
2014-08-26 13:58           ` [Cluster-devel] " Christoph Hellwig
2014-08-23 14:41 ` [PATCH 08/10] locks: move i_lock acquisition into generic_*_lease handlers Jeff Layton
2014-08-23 14:41   ` [Cluster-devel] " Jeff Layton
     [not found]   ` <1408804878-1331-9-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 16:06     ` Christoph Hellwig
2014-08-24 16:06       ` [Cluster-devel] " Christoph Hellwig
2014-08-24 16:06       ` Christoph Hellwig
     [not found]       ` <20140824160634.GG15908-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-24 16:11         ` Christoph Hellwig
2014-08-24 16:11           ` [Cluster-devel] " Christoph Hellwig
2014-08-24 16:11           ` Christoph Hellwig
     [not found]           ` <20140824161134.GJ15908-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-31 14:51             ` Jeff Layton
2014-08-31 14:51               ` [Cluster-devel] " Jeff Layton
2014-08-31 14:51               ` Jeff Layton
2014-08-25  1:36         ` Jeff Layton
2014-08-25  1:36           ` [Cluster-devel] " Jeff Layton
2014-08-25  1:36           ` Jeff Layton
     [not found] ` <1408804878-1331-1-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-23 14:41   ` [PATCH 01/10] locks: close potential race in lease_get_mtime Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-2-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 15:48       ` Christoph Hellwig
2014-08-24 15:48         ` [Cluster-devel] " Christoph Hellwig
2014-08-24 15:48         ` Christoph Hellwig
2014-08-25 20:01     ` J. Bruce Fields
2014-08-25 20:01       ` [Cluster-devel] " J. Bruce Fields
2014-08-23 14:41   ` Jeff Layton [this message]
2014-08-23 14:41     ` [Cluster-devel] [PATCH 02/10] nfsd: fix potential lease memory leak in nfs4_setlease Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-3-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 15:48       ` Christoph Hellwig
2014-08-24 15:48         ` [Cluster-devel] " Christoph Hellwig
2014-08-24 15:48         ` Christoph Hellwig
2014-08-23 14:41   ` [PATCH 03/10] locks: generic_delete_lease doesn't need a file_lock at all Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-4-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24  1:27       ` Christoph Hellwig
2014-08-24  1:27         ` [Cluster-devel] " Christoph Hellwig
2014-08-24  1:27         ` Christoph Hellwig
     [not found]         ` <20140824012757.GA21609-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-24 10:09           ` Jeff Layton
2014-08-24 10:09             ` [Cluster-devel] " Jeff Layton
2014-08-24 10:09             ` Jeff Layton
2014-08-23 14:41   ` [PATCH 05/10] nfsd: don't keep a pointer to the lease in nfs4_file Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-6-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 15:51       ` Christoph Hellwig
2014-08-24 15:51         ` [Cluster-devel] " Christoph Hellwig
2014-08-24 15:51         ` Christoph Hellwig
2014-08-23 14:41   ` [PATCH 06/10] locks: plumb an "aux" pointer into the setlease routines Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-7-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24  1:33       ` Christoph Hellwig
2014-08-24  1:33         ` [Cluster-devel] " Christoph Hellwig
2014-08-24  1:33         ` Christoph Hellwig
     [not found]         ` <20140824013305.GB21609-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-24 10:08           ` Jeff Layton
2014-08-24 10:08             ` [Cluster-devel] " Jeff Layton
2014-08-24 10:08             ` Jeff Layton
     [not found]             ` <20140824060801.5402880c-08S845evdOaAjSkqwZiSMmfYqLom42DlXqFh9Ls21Oc@public.gmane.org>
2014-08-24 15:54               ` Christoph Hellwig
2014-08-24 15:54                 ` [Cluster-devel] " Christoph Hellwig
2014-08-24 15:54                 ` Christoph Hellwig
2014-08-25 20:28       ` J. Bruce Fields
2014-08-25 20:28         ` [Cluster-devel] " J. Bruce Fields
2014-08-25 20:28         ` J. Bruce Fields
     [not found]         ` <20140825202852.GD21957-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2014-08-26 10:53           ` Jeff Layton
2014-08-26 10:53             ` [Cluster-devel] " Jeff Layton
2014-08-26 10:53             ` Jeff Layton
2014-08-23 14:41   ` [PATCH 09/10] locks: move freeing of leases outside of i_lock Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
     [not found]     ` <1408804878-1331-10-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
2014-08-24 16:08       ` Christoph Hellwig
2014-08-24 16:08         ` [Cluster-devel] " Christoph Hellwig
2014-08-24 16:08         ` Christoph Hellwig
     [not found]         ` <20140824160804.GH15908-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-25  1:35           ` Jeff Layton
2014-08-25  1:35             ` [Cluster-devel] " Jeff Layton
2014-08-25  1:35             ` Jeff Layton
2014-08-23 14:41   ` [PATCH 10/10] locks: update Documentation/filesystems with lease API changes Jeff Layton
2014-08-23 14:41     ` [Cluster-devel] " Jeff Layton
2014-08-23 14:41     ` Jeff Layton
2014-08-24 16:10   ` [PATCH 00/10] locks/nfsd: internal lease API overhaul Christoph Hellwig
2014-08-24 16:10     ` [Cluster-devel] " Christoph Hellwig
2014-08-24 16:10     ` Christoph Hellwig
     [not found]     ` <20140824161046.GI15908-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-08-25  1:43       ` Jeff Layton
2014-08-25  1:43         ` [Cluster-devel] " Jeff Layton
2014-08-25  1:43         ` Jeff Layton
     [not found]         ` <20140824214301.61019123-08S845evdOaAjSkqwZiSMmfYqLom42DlXqFh9Ls21Oc@public.gmane.org>
2014-08-26 13:59           ` Christoph Hellwig
2014-08-26 13:59             ` [Cluster-devel] " Christoph Hellwig
2014-08-26 13:59             ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1408804878-1331-3-git-send-email-jlayton@primarydata.com \
    --to=jlayton-7i+n7zu2hftekmmhf/gkza@public.gmane.org \
    --cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
    --cc=cluster-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.