From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752607Ab1LZSiZ (ORCPT ); Mon, 26 Dec 2011 13:38:25 -0500 Received: from mail-iy0-f174.google.com ([209.85.210.174]:61573 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751345Ab1LZSiX (ORCPT ); Mon, 26 Dec 2011 13:38:23 -0500 Date: Mon, 26 Dec 2011 10:37:56 -0800 (PST) From: Linus Torvalds X-X-Sender: torvalds@i5.linux-foundation.org To: "J. Bruce Fields" cc: Al Viro , Linus Torvalds , linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: file locking fix for 3.2 In-Reply-To: <20111225181948.GA26288@fieldses.org> Message-ID: References: <20111224215012.GA23495@fieldses.org> <20111224225525.GR23916@ZenIV.linux.org.uk> <20111224235035.GA23711@fieldses.org> <20111225000542.GS23916@ZenIV.linux.org.uk> <20111225181948.GA26288@fieldses.org> User-Agent: Alpine 2.02 (LFD 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I'm committing the appended version instead. It removes all the games with IS_ERR(), and just does the checking at the allocation point, the way the other lease_alloc() user does too. Holler if you see something odd in there.. Linus --- commit 6d4b9e38d3980826abccfbd90e95bf4bd41b8dd2 Author: Linus Torvalds Date: Mon Dec 26 10:25:26 2011 -0800 vfs: fix handling of lock allocation failure in lease-break case Bruce Fields notes that commit 778fc546f749 ("locks: fix tracking of inprogress lease breaks") introduced a possible error pointer dereference on failure to allocate memory. locks_conflict() will dereference the passed-in new lease lock structure that may be an error pointer. This means an open (without O_NONBLOCK set) on a file with a lease applied (generally only done when Samba or nfsd (with v4) is running) could crash if a kmalloc() fails. So instead of playing games with IS_ERROR() all over the place, just check the allocation failure early. That makes the code more straightforward, and avoids this possible bad pointer dereference. Based-on-patch-by: J. Bruce Fields Cc: Al Viro Signed-off-by: Linus Torvalds --- fs/locks.c | 11 +++-------- 1 files changed, 3 insertions(+), 8 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 3b0d05dcd7c1..637694bf3a03 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1205,6 +1205,8 @@ int __break_lease(struct inode *inode, unsigned int mode) int want_write = (mode & O_ACCMODE) != O_RDONLY; new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK); + if (IS_ERR(new_fl)) + return PTR_ERR(new_fl); lock_flocks(); @@ -1221,12 +1223,6 @@ int __break_lease(struct inode *inode, unsigned int mode) if (fl->fl_owner == current->files) i_have_this_lease = 1; - if (IS_ERR(new_fl) && !i_have_this_lease - && ((mode & O_NONBLOCK) == 0)) { - error = PTR_ERR(new_fl); - goto out; - } - break_time = 0; if (lease_break_time > 0) { break_time = jiffies + lease_break_time * HZ; @@ -1284,8 +1280,7 @@ restart: out: unlock_flocks(); - if (!IS_ERR(new_fl)) - locks_free_lock(new_fl); + locks_free_lock(new_fl); return error; }