public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: stable-review@kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	"Theodore Tso" <tytso@mit.edu>,
	Curt Wohlgemuth <curtw@google.com>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: [05/34] ext4: fix potential buffer head leak when add_dirent_to_buf() returns ENOSPC
Date: Thu, 10 Dec 2009 21:23:17 -0800	[thread overview]
Message-ID: <20091211052542.664737460@linux.site> (raw)
In-Reply-To: <20091211052858.GA23229@kroah.com>

[-- Attachment #1: 0001-ext4-fix-potential-buffer-head-leak-when-add_dirent_.patch --]
[-- Type: text/plain, Size: 3934 bytes --]

2.6.32-stable review patch.  If anyone has any objections, please let us know.

------------------

(cherry picked from commit 2de770a406b06dfc619faabbf5d85c835ed3f2e1)

Previously add_dirent_to_buf() did not free its passed-in buffer head
in the case of ENOSPC, since in some cases the caller still needed it.
However, this led to potential buffer head leaks since not all callers
dealt with this correctly.  Fix this by making simplifying the freeing
convention; now add_dirent_to_buf() *never* frees the passed-in buffer
head, and leaves that to the responsibility of its caller.  This makes
things cleaner and easier to prove that the code is neither leaking
buffer heads or calling brelse() one time too many.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Curt Wohlgemuth <curtw@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 fs/ext4/namei.c |   30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1292,9 +1292,6 @@ errout:
  * add_dirent_to_buf will attempt search the directory block for
  * space.  It will return -ENOSPC if no space is available, and -EIO
  * and -EEXIST if directory entry already exists.
- *
- * NOTE!  bh is NOT released in the case where ENOSPC is returned.  In
- * all other cases bh is released.
  */
 static int add_dirent_to_buf(handle_t *handle, struct dentry *dentry,
 			     struct inode *inode, struct ext4_dir_entry_2 *de,
@@ -1315,14 +1312,10 @@ static int add_dirent_to_buf(handle_t *h
 		top = bh->b_data + blocksize - reclen;
 		while ((char *) de <= top) {
 			if (!ext4_check_dir_entry("ext4_add_entry", dir, de,
-						  bh, offset)) {
-				brelse(bh);
+						  bh, offset))
 				return -EIO;
-			}
-			if (ext4_match(namelen, name, de)) {
-				brelse(bh);
+			if (ext4_match(namelen, name, de))
 				return -EEXIST;
-			}
 			nlen = EXT4_DIR_REC_LEN(de->name_len);
 			rlen = ext4_rec_len_from_disk(de->rec_len, blocksize);
 			if ((de->inode? rlen - nlen: rlen) >= reclen)
@@ -1337,7 +1330,6 @@ static int add_dirent_to_buf(handle_t *h
 	err = ext4_journal_get_write_access(handle, bh);
 	if (err) {
 		ext4_std_error(dir->i_sb, err);
-		brelse(bh);
 		return err;
 	}
 
@@ -1377,7 +1369,6 @@ static int add_dirent_to_buf(handle_t *h
 	err = ext4_handle_dirty_metadata(handle, dir, bh);
 	if (err)
 		ext4_std_error(dir->i_sb, err);
-	brelse(bh);
 	return 0;
 }
 
@@ -1471,7 +1462,9 @@ static int make_indexed_dir(handle_t *ha
 	if (!(de))
 		return retval;
 
-	return add_dirent_to_buf(handle, dentry, inode, de, bh);
+	retval = add_dirent_to_buf(handle, dentry, inode, de, bh);
+	brelse(bh);
+	return retval;
 }
 
 /*
@@ -1514,8 +1507,10 @@ static int ext4_add_entry(handle_t *hand
 		if(!bh)
 			return retval;
 		retval = add_dirent_to_buf(handle, dentry, inode, NULL, bh);
-		if (retval != -ENOSPC)
+		if (retval != -ENOSPC) {
+			brelse(bh);
 			return retval;
+		}
 
 		if (blocks == 1 && !dx_fallback &&
 		    EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_DIR_INDEX))
@@ -1528,7 +1523,9 @@ static int ext4_add_entry(handle_t *hand
 	de = (struct ext4_dir_entry_2 *) bh->b_data;
 	de->inode = 0;
 	de->rec_len = ext4_rec_len_to_disk(blocksize, blocksize);
-	return add_dirent_to_buf(handle, dentry, inode, de, bh);
+	retval = add_dirent_to_buf(handle, dentry, inode, de, bh);
+	brelse(bh);
+	return retval;
 }
 
 /*
@@ -1561,10 +1558,8 @@ static int ext4_dx_add_entry(handle_t *h
 		goto journal_error;
 
 	err = add_dirent_to_buf(handle, dentry, inode, NULL, bh);
-	if (err != -ENOSPC) {
-		bh = NULL;
+	if (err != -ENOSPC)
 		goto cleanup;
-	}
 
 	/* Block full, should compress but for now just split */
 	dxtrace(printk(KERN_DEBUG "using %u of %u node entries\n",
@@ -1657,7 +1652,6 @@ static int ext4_dx_add_entry(handle_t *h
 	if (!de)
 		goto cleanup;
 	err = add_dirent_to_buf(handle, dentry, inode, de, bh);
-	bh = NULL;
 	goto cleanup;
 
 journal_error:



  parent reply	other threads:[~2009-12-11  5:29 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20091211052312.805428372@linux.site>
2009-12-11  5:28 ` [00/34] 2.6.32.1-stable review Greg KH
2009-12-11  5:23   ` [01/34] signal: Fix alternate signal stack check Greg KH
2009-12-11  5:23   ` [02/34] SCSI: scsi_lib_dma: fix bug with dma maps on nested scsi objects Greg KH
2009-12-11  5:23   ` [03/34] SCSI: osd_protocol.h: Add missing #include Greg KH
2009-12-11  5:23   ` [04/34] SCSI: megaraid_sas: fix 64 bit sense pointer truncation Greg KH
2009-12-11  5:23   ` Greg KH [this message]
2009-12-11  5:23   ` [06/34] ext4: avoid divide by zero when trying to mount a corrupted file system Greg KH
2009-12-11  5:23   ` [07/34] ext4: fix the returned block count if EXT4_IOC_MOVE_EXT fails Greg KH
2009-12-11  5:23   ` [08/34] ext4: fix lock order problem in ext4_move_extents() Greg KH
2009-12-11  5:23   ` [09/34] ext4: fix possible recursive locking warning in EXT4_IOC_MOVE_EXT Greg KH
2009-12-11  5:23   ` [10/34] ext4: plug a buffer_head leak in an error path of ext4_iget() Greg KH
2009-12-11  5:23   ` [11/34] ext4: make sure directory and symlink blocks are revoked Greg KH
2009-12-11  5:23   ` [12/34] ext4: fix i_flags access in ext4_da_writepages_trans_blocks() Greg KH
2009-12-11  5:23   ` [13/34] ext4: journal all modifications in ext4_xattr_set_handle Greg KH
2009-12-11  5:23   ` [14/34] ext4: dont update the superblock in ext4_statfs() Greg KH
2009-12-11  5:23   ` [15/34] ext4: fix uninit block bitmap initialization when s_meta_first_bg is non-zero Greg KH
2009-12-11  5:23   ` [16/34] ext4: fix block validity checks so they work correctly with meta_bg Greg KH
2009-12-11  5:23   ` [17/34] ext4: avoid issuing unnecessary barriers Greg KH
2009-12-11  5:23   ` [18/34] ext4: fix error handling in ext4_ind_get_blocks() Greg KH
2009-12-11  5:23   ` [19/34] ext4: make trim/discard optional (and off by default) Greg KH
2009-12-11  5:23   ` [20/34] ext4: make "norecovery" an alias for "noload" Greg KH
2009-12-11  5:23   ` [21/34] ext4: Fix double-free of blocks with EXT4_IOC_MOVE_EXT Greg KH
2009-12-11  5:23   ` [22/34] ext4: initialize moved_len before calling ext4_move_extents() Greg KH
2009-12-11  5:23   ` [23/34] ext4: move_extent_per_page() cleanup Greg KH
2009-12-11  5:23   ` [24/34] jbd2: Add ENOMEM checking in and for jbd2_journal_write_metadata_buffer() Greg KH
2009-12-11  5:23   ` [25/34] ext4: Return the PTR_ERR of the correct pointer in setup_new_group_blocks() Greg KH
2009-12-11  5:23   ` [26/34] ext4: Avoid data / filesystem corruption when write fails to copy data Greg KH
2009-12-11  5:23   ` [27/34] ext4: wait for log to commit when umounting Greg KH
2009-12-11  5:23   ` [28/34] ext4: remove blocks from inode prealloc list on failure Greg KH
2009-12-11  5:23   ` [29/34] ext4: ext4_get_reserved_space() must return bytes instead of blocks Greg KH
2009-12-11  5:23   ` [30/34] ext4: quota macros cleanup Greg KH
2009-12-11  5:23   ` [31/34] ext4: fix incorrect block reservation on quota transfer Greg KH
2009-12-11  5:23   ` [32/34] ext4: Wait for proper transaction commit on fsync Greg KH
2009-12-11  5:23   ` [33/34] ext4: Fix insufficient checks in EXT4_IOC_MOVE_EXT Greg KH
2009-12-11  5:23   ` [34/34] ext4: Fix potential fiemap deadlock (mmap_sem vs. i_data_sem) Greg KH

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=20091211052542.664737460@linux.site \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=curtw@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable-review@kernel.org \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox