linux-ext4 - [PATCH] fix file system corruption [ was Re: mballoc errors ]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 1 Apr 2008 02:20:30 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Valerie Clement <valerie.clement@...l.net>
Cc:	linux-ext4 <linux-ext4@...r.kernel.org>
Subject: [PATCH] fix file system corruption [ was Re: mballoc errors ]

On Tue, Apr 01, 2008 at 01:48:19AM +0530, Aneesh Kumar K.V wrote:
> On Mon, Mar 31, 2008 at 04:58:06PM +0200, Valerie Clement wrote:
> 
> I actually added fallocate to fsstress and created the file filesystem
> as you suggested. I am able to reproduce the problem once. Currently
> doing a code audit. Will let you know if i make any progress.
> 

I found one case where it could fail. We are returning total extent size
in case of converting from uninitialized extent to initialized one. 
Now if we have the below layout

[p --------x--------]

where p is the start block and x is the location where we intent to
write. If converting the above uninit extent resulted in 

[p zeroed-out][uninit].

We should not be returning the size of
zeroed-out-extent. That is because in ext4_ext_get_blocks we cache
the extent information as below

out_new:
		....
/* Cache only when it is _not_ an uninitialized extent */
if (create != EXT4_CREATE_UNINITIALIZED_EXT)
	ext4_ext_put_in_cache(inode, iblock, allocated, newblock,
						EXT4_EXT_CACHE_EXTENT);
out:


That would mean we are caching an extent starting from newblock of
size allocated where allocated would be the size of zeroout extent,
which is actually wrong.

I am yet to test the patch. If you have time can you test the below
change

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 964d2c1..91f8f72 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2264,7 +2264,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 		ex->ee_len   = orig_ex.ee_len;
 		ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 		ext4_ext_dirty(handle, inode, path + depth);
-		return le16_to_cpu(ex->ee_len);
+		/* zeroed the full extent */
+		return allocated;
 	}
 
 	/* ex1: ee_block to iblock - 1 : uninitialized */
@@ -2309,11 +2310,13 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 				ex->ee_len   = orig_ex.ee_len;
 				ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 				ext4_ext_dirty(handle, inode, path + depth);
-				return le16_to_cpu(ex->ee_len);
+				/* zeroed the full extent */
+				return allocated;
 
 			} else if (err)
 				goto fix_extent_len;
 
+			/* zeroed the second half */
 			return allocated;
 		}
 		ex3 = &newex;
@@ -2331,7 +2334,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			ex->ee_len   = orig_ex.ee_len;
 			ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 			ext4_ext_dirty(handle, inode, path + depth);
-			return le16_to_cpu(ex->ee_len);
+			/* zeroed the full extent */
+			return allocated;
 
 		} else if (err)
 			goto fix_extent_len;
@@ -2379,7 +2383,8 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			ex->ee_len   = orig_ex.ee_len;
 			ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 			ext4_ext_dirty(handle, inode, path + depth);
-			return le16_to_cpu(ex->ee_len);
+			/* zero out the first half */
+			return allocated;
 		}
 	}
 	/*
@@ -2446,7 +2451,8 @@ insert:
 		ex->ee_len   = orig_ex.ee_len;
 		ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
 		ext4_ext_dirty(handle, inode, path + depth);
-		return le16_to_cpu(ex->ee_len);
+		/* zero out the first half */
+		return allocated;
 	} else if (err)
 		goto fix_extent_len;
 out:
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html