linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leonard Michlmayr <leonard.michlmayr@gmail.com>
To: Akira Fujita <a-fujita@rs.jp.nec.com>, tytso@mit.edu
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: [incomplete PATCH] ext4: avoid overflow in fiemap
Date: Thu, 04 Mar 2010 23:28:35 +0100	[thread overview]
Message-ID: <1267741715.13886.4.camel@michlmayr> (raw)
In-Reply-To: <1267739098.4204.12.camel@michlmayr>

A __u32 cannot hold the maximum number of blocks in a file. This may
lead to an overflow if a fiemap request has length = s_maxbytes.

I still get a segfault, I have to go through it again. This is just for
your information. I will work on it tomorrow.

The patch is to be applied after/at the end of the ext4 patch queue.
Appearently only fiemap uses ext4_ext_walk_space(). Therefore I changed
the semantics of the ext4_ext_walk_space() call. I hope this does not
introduce a new problem.

In the case that 1<<32 blocks are requested, but there are no allocated
extents, I return two gaps because, the length of a gap in the
ext4_ext_cache is a __u32 again.

Regards
Leonard 

Signed-off-by: Leonard Michlmayr <leonard.michlmayr@gmail.com>

diff -puar linux-2.6.33-ted/fs/ext4/extents.c linux-2.6.33-lm/fs/ext4/extents.c
--- linux-2.6.33-ted/fs/ext4/extents.c	2010-03-04 21:08:00.000000000 +0100
+++ linux-2.6.33-lm/fs/ext4/extents.c	2010-03-04 22:50:54.000000000 +0100
@@ -1853,21 +1853,21 @@ cleanup:
 }
 
 int ext4_ext_walk_space(struct inode *inode, ext4_lblk_t block,
-			ext4_lblk_t num, ext_prepare_callback func,
+			ext4_lblk_t last, ext_prepare_callback func,
 			void *cbdata)
 {
 	struct ext4_ext_path *path = NULL;
 	struct ext4_ext_cache cbex;
 	struct ext4_extent *ex;
 	ext4_lblk_t next, start = 0, end = 0;
-	ext4_lblk_t last = block + num;
 	int depth, exists, err = 0;
 
 	BUG_ON(func == NULL);
 	BUG_ON(inode == NULL);
 
-	while (block < last && block != EXT_MAX_BLOCK) {
-		num = last - block;
+	while (block <= last && block != EXT_MAX_BLOCK) {
+		/* num is one less than the number of blocks */
+		ext4_lblk_t num = last - block;
 		/* find extent for this block */
 		down_read(&EXT4_I(inode)->i_data_sem);
 		path = ext4_ext_find_extent(inode, block, path);
@@ -1893,10 +1893,12 @@ int ext4_ext_walk_space(struct inode *in
 			 * all requested space */
 			start = block;
 			end = block + num;
+			if (end - start >= EXT_MAX_BLOCK)
+				end = EXT_MAX_BLOCK - 1;
 		} else if (le32_to_cpu(ex->ee_block) > block) {
 			/* need to allocate space before found extent */
 			start = block;
-			end = le32_to_cpu(ex->ee_block);
+			end = le32_to_cpu(ex->ee_block) - 1;
 			if (block + num < end)
 				end = block + num;
 		} else if (block >= le32_to_cpu(ex->ee_block)
@@ -1904,8 +1906,11 @@ int ext4_ext_walk_space(struct inode *in
 			/* need to allocate space after found extent */
 			start = block;
 			end = block + num;
+			if (end - start >= EXT_MAX_BLOCK)
+				end = EXT_MAX_BLOCK - 1;
+			BUG_ON(next == 0);
 			if (end >= next)
-				end = next;
+				end = next - 1;
 		} else if (block >= le32_to_cpu(ex->ee_block)) {
 			/*
 			 * some part of requested space is covered
@@ -1913,7 +1918,7 @@ int ext4_ext_walk_space(struct inode *in
 			 */
 			start = block;
 			end = le32_to_cpu(ex->ee_block)
-				+ ext4_ext_get_actual_len(ex);
+				+ (ext4_ext_get_actual_len(ex) - 1);
 			if (block + num < end)
 				end = block + num;
 			exists = 1;
@@ -1924,7 +1929,7 @@ int ext4_ext_walk_space(struct inode *in
 
 		if (!exists) {
 			cbex.ec_block = start;
-			cbex.ec_len = end - start;
+			cbex.ec_len = end - start + 1;
 			cbex.ec_start = 0;
 			cbex.ec_type = EXT4_EXT_CACHE_GAP;
 		} else {
@@ -3894,7 +3899,6 @@ static int ext4_xattr_fiemap(struct inod
 int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		__u64 start, __u64 len)
 {
-	ext4_lblk_t start_blk;
 	int error = 0;
 
 	/* fallback to generic here if not in extents fmt */
@@ -3908,17 +3912,16 @@ int ext4_fiemap(struct inode *inode, str
 	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
 		error = ext4_xattr_fiemap(inode, fieinfo);
 	} else {
-		ext4_lblk_t last_blk, len_blks;
+		ext4_lblk_t start_blk, last_blk;
 
 		start_blk = start >> inode->i_sb->s_blocksize_bits;
-		last_blk = (start + len - 1) >> inode->i_sb->s_blocksize_bits;
-		len_blks = last_blk - start_blk + 1;
+		last_blk = (len - 1 + start) >> inode->i_sb->s_blocksize_bits;
 
 		/*
 		 * Walk the extent tree gathering extent information.
 		 * ext4_ext_fiemap_cb will push extents back to user.
 		 */
-		error = ext4_ext_walk_space(inode, start_blk, len_blks,
+		error = ext4_ext_walk_space(inode, start_blk, last_blk,
 					  ext4_ext_fiemap_cb, fieinfo);
 	}
 




  reply	other threads:[~2010-03-04 22:28 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-02 18:18 [PATCH 00/28] Ext4 patch queue for the 2.6.34 merge window Theodore Ts'o
2010-03-02 18:18 ` [PATCH 01/28] ext4: fix async i/o writes beyond 4GB to a sparse file Theodore Ts'o
2010-03-02 18:18 ` [PATCH 02/28] ext4: Fix optional-arg mount options Theodore Ts'o
2010-03-02 18:18 ` [PATCH 03/28] ext4: Add block validity check when truncating indirect block mapped inodes Theodore Ts'o
2010-03-02 18:18 ` [PATCH 04/28] ext4: Add new tracepoint for jbd2_cleanup_journal_tail Theodore Ts'o
2010-03-02 18:18 ` [PATCH 05/28] ext4: Add new tracepoints to debug delayed allocation space functions Theodore Ts'o
2010-03-02 18:18 ` [PATCH 06/28] ext4: Use slab allocator for sub-page sized allocations Theodore Ts'o
2010-03-02 18:18 ` [PATCH 07/28] ext4: Use bitops to read/modify EXT4_I(inode)->i_state Theodore Ts'o
2010-03-02 18:18 ` [PATCH 08/28] ext4: Reserve INCOMPAT_EA_INODE and INCOMPAT_DIRDATA feature codepoints Theodore Ts'o
2010-03-02 18:18 ` [PATCH 09/28] ext4: move __func__ into a macro for ext4_warning, ext4_error Theodore Ts'o
2010-03-02 18:18 ` [PATCH 10/28] ext4: add missing error checking to ext4_expand_extra_isize_ea() Theodore Ts'o
2010-03-02 18:18 ` [PATCH 11/28] ext4: correctly calculate number of blocks for fiemap Theodore Ts'o
2010-03-03  7:47   ` Akira Fujita
2010-03-03  8:34     ` Leonard Michlmayr
2010-03-03 17:52     ` tytso
2010-03-04  5:40       ` Akira Fujita
2010-03-04 21:44         ` Leonard Michlmayr
2010-03-04 22:28           ` Leonard Michlmayr [this message]
2010-03-07  3:38             ` [incomplete PATCH] ext4: avoid overflow in fiemap tytso
2010-03-04 23:38           ` [PATCH 11/28] ext4: correctly calculate number of blocks for fiemap Eric Sandeen
2010-03-05 16:46             ` Leonard Michlmayr
2010-03-04 22:08         ` tytso
2010-03-04 23:47           ` Eric Sandeen
2010-03-02 18:18 ` [PATCH 12/28] jbd2: delay discarding buffers in journal_unmap_buffer Theodore Ts'o
2010-03-02 18:18 ` [PATCH 13/28] ext4: Fix BUG_ON at fs/buffer.c:652 in no journal mode Theodore Ts'o
2010-03-02 18:18 ` [PATCH 14/28] ext4: Add flag to files with blocks intentionally past EOF Theodore Ts'o
2010-03-02 18:18 ` [PATCH 15/28] ext4: mount flags manipulation cleanup Theodore Ts'o
2010-03-02 18:18 ` [PATCH 16/28] ext4: trivial quota cleanup Theodore Ts'o
2010-03-02 18:18 ` [PATCH 17/28] jbd2: clean up an assertion in jbd2_journal_commit_transaction() Theodore Ts'o
2010-03-02 18:18 ` [PATCH 18/28] ext4: Fix fencepost error in chosing choosing group vs file preallocation Theodore Ts'o
2010-03-02 18:18 ` [PATCH 19/28] ext4: deprecate obsoleted mount options Theodore Ts'o
2010-03-02 18:18 ` [PATCH 20/28] ext4: fix error handling in migrate Theodore Ts'o
2010-03-02 18:18 ` [PATCH 21/28] ext4: explicitly remove inode from orphan list after failed direct io Theodore Ts'o
2010-03-02 18:18 ` [PATCH 22/28] ext4: Handle non empty on-disk orphan link Theodore Ts'o
2010-03-02 18:18 ` [PATCH 23/28] ext4: make "offset" consistent in ext4_check_dir_entry() Theodore Ts'o
2010-03-02 18:18 ` [PATCH 24/28] ext4: mechanical change on dio get_block code in prepare for it to be used by buffer write Theodore Ts'o
2010-03-02 18:18 ` [PATCH 25/28] ext4: use ext4_get_block_write in " Theodore Ts'o
2010-03-02 18:18 ` [PATCH 26/28] ext4: Use direct_IO_no_locking in ext4 dio read Theodore Ts'o
2010-03-02 18:18 ` [PATCH 27/28] ext4: Convert BUG_ON checks to use ext4_error() instead Theodore Ts'o
2010-03-06 13:03   ` Aneesh Kumar K. V
2010-03-07  2:45     ` tytso
2010-03-07  5:51       ` Eric Sandeen
2010-03-07 17:36         ` Andreas Dilger
2010-03-08 17:46       ` Frank Mayhar
2010-03-02 18:18 ` [PATCH 28/28] ext4: Fix ext4_quota_write cross block boundary behaviour Theodore Ts'o
2010-03-02 18:26 ` [PATCH 00/28] Ext4 patch queue for the 2.6.34 merge window tytso

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=1267741715.13886.4.camel@michlmayr \
    --to=leonard.michlmayr@gmail.com \
    --cc=a-fujita@rs.jp.nec.com \
    --cc=linux-ext4@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).