Patchwork [4/6] e2fsprogs: properly set up extent header in do_write

login
register
mail settings
Submitter Robert Yang
Date Aug. 22, 2013, 1:13 p.m.
Message ID <37674ce0aeae479e3cde7f578871ed47ec9025f8.1377175027.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/56255/
State Accepted
Commit 7d1e51681d25f6e6d2c20744825723ad5c83861c
Headers show

Comments

Robert Yang - Aug. 22, 2013, 1:13 p.m.
do_write doesn't fully set up the first extent header on a new
inode, so if we write a 0-length file, and don't write any data
to the new file, we end up creating something that looks corrupt
to kernelspace:

EXT4-fs error (device loop0): ext4_ext_check_inode:464: inode #12: comm
ls: bad header/extent: invalid magic - magic 0, entries 0, max 0(0),
depth 0(0)

Do something similar to ext4_ext_tree_init() here, and
fill out the first extent header upon creation to avoid this.

[YOCTO #3848]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 .../e2fsprogs-1.42.8/debugfs-extent-header.patch   |   47 ++++++++++++++++++++
 .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    2 +
 2 files changed, 49 insertions(+)
 create mode 100644 meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-extent-header.patch
Darren Hart - Aug. 22, 2013, 5:24 p.m.
On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
> do_write doesn't fully set up the first extent header on a new
> inode, so if we write a 0-length file, and don't write any data
> to the new file, we end up creating something that looks corrupt
> to kernelspace:
> 
> EXT4-fs error (device loop0): ext4_ext_check_inode:464: inode #12: comm
> ls: bad header/extent: invalid magic - magic 0, entries 0, max 0(0),
> depth 0(0)
> 
> Do something similar to ext4_ext_tree_init() here, and
> fill out the first extent header upon creation to avoid this.
> 
> [YOCTO #3848]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  .../e2fsprogs-1.42.8/debugfs-extent-header.patch   |   47 ++++++++++++++++++++
>  .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    2 +

> +Upstream-Status: Backport

Should we backport? Or should we just update the revision we use?
Robert Yang - Aug. 23, 2013, 2:02 a.m.
On 08/23/2013 01:24 AM, Darren Hart wrote:
> On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
>> do_write doesn't fully set up the first extent header on a new
>> inode, so if we write a 0-length file, and don't write any data
>> to the new file, we end up creating something that looks corrupt
>> to kernelspace:
>>
>> EXT4-fs error (device loop0): ext4_ext_check_inode:464: inode #12: comm
>> ls: bad header/extent: invalid magic - magic 0, entries 0, max 0(0),
>> depth 0(0)
>>
>> Do something similar to ext4_ext_tree_init() here, and
>> fill out the first extent header upon creation to avoid this.
>>
>> [YOCTO #3848]
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   .../e2fsprogs-1.42.8/debugfs-extent-header.patch   |   47 ++++++++++++++++++++
>>   .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    2 +
>
>> +Upstream-Status: Backport
>
> Should we backport? Or should we just update the revision we use?
>

Yes, I think so, Ted said that he had merge this patch a few days ago, but
I didn't see where is it, I pulled this patch from the linux ext mailing
list.

// Robert
Darren Hart - Aug. 23, 2013, 5:04 p.m.
On Fri, 2013-08-23 at 10:02 +0800, Robert Yang wrote:
> On 08/23/2013 01:24 AM, Darren Hart wrote:
> > On Thu, 2013-08-22 at 09:13 -0400, Robert Yang wrote:
> >> do_write doesn't fully set up the first extent header on a new
> >> inode, so if we write a 0-length file, and don't write any data
> >> to the new file, we end up creating something that looks corrupt
> >> to kernelspace:
> >>
> >> EXT4-fs error (device loop0): ext4_ext_check_inode:464: inode #12: comm
> >> ls: bad header/extent: invalid magic - magic 0, entries 0, max 0(0),
> >> depth 0(0)
> >>
> >> Do something similar to ext4_ext_tree_init() here, and
> >> fill out the first extent header upon creation to avoid this.
> >>
> >> [YOCTO #3848]
> >>
> >> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> >> ---
> >>   .../e2fsprogs-1.42.8/debugfs-extent-header.patch   |   47 ++++++++++++++++++++
> >>   .../recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb |    2 +
> >
> >> +Upstream-Status: Backport
> >
> > Should we backport? Or should we just update the revision we use?
> >
> 
> Yes, I think so, Ted said that he had merge this patch a few days ago, but
> I didn't see where is it, I pulled this patch from the linux ext mailing
> list.

OK, yeah, this is fine. It will be nice to rebase at some point in the
future and drop these patches, but we have to do that anyway, so this is
fine in my opinion.

Acked-by: Darren Hart <dvhart@linux.intel.com>

Patch

diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-extent-header.patch b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-extent-header.patch
new file mode 100644
index 0000000..ae44730
--- /dev/null
+++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs-1.42.8/debugfs-extent-header.patch
@@ -0,0 +1,47 @@ 
+debugfs: properly set up extent header in do_write
+
+do_write doesn't fully set up the first extent header on a new
+inode, so if we write a 0-length file, and don't write any data
+to the new file, we end up creating something that looks corrupt
+to kernelspace:
+
+EXT4-fs error (device loop0): ext4_ext_check_inode:464: inode #12: comm ls: bad header/extent: invalid magic - magic 0, entries 0, max 0(0), depth 0(0)
+
+Do something similar to ext4_ext_tree_init() here, and
+fill out the first extent header upon creation to avoid this.
+
+Upstream-Status: Backport
+
+Reported-by: Robert Yang <liezhi.yang@windriver.com>
+Signed-off-by: Eric Sandeen <sandeen@redhat.com>
+---
+ debugfs/debugfs.c | 13 ++++++++++++-
+ 1 file changed, 12 insertions(+), 1 deletion(-)
+
+diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
+--- a/debugfs/debugfs.c
++++ b/debugfs/debugfs.c
+@@ -1726,8 +1726,19 @@ void do_write(int argc, char *argv[])
+ 	inode.i_links_count = 1;
+ 	inode.i_size = statbuf.st_size;
+ 	if (current_fs->super->s_feature_incompat &
+-	    EXT3_FEATURE_INCOMPAT_EXTENTS)
++	    EXT3_FEATURE_INCOMPAT_EXTENTS) {
++		int i;
++		struct ext3_extent_header *eh;
++
++		eh = (struct ext3_extent_header *) &inode.i_block[0];
++		eh->eh_depth = 0;
++		eh->eh_entries = 0;
++		eh->eh_magic = EXT3_EXT_MAGIC;
++		i = (sizeof(inode.i_block) - sizeof(*eh)) /
++			sizeof(struct ext3_extent);
++		eh->eh_max = ext2fs_cpu_to_le16(i);
+ 		inode.i_flags |= EXT4_EXTENTS_FL;
++	}
+ 	if (debugfs_write_new_inode(newfile, &inode, argv[0])) {
+ 		close(fd);
+ 		return;
+-- 
+1.8.1.2
+
diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
index b063bc0..2dc9dab 100644
--- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
+++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.42.8.bb
@@ -6,6 +6,8 @@  SRC_URI += "file://acinclude.m4 \
             file://debugfs-too-short.patch \
             file://debugfs-sparse-copy.patch \
             file://fix-icache.patch \
+            file://debugfs-extent-header.patch \
+            file://populate-extfs.sh \
 "
 
 SRC_URI[md5sum] = "8ef664b6eb698aa6b733df59b17b9ed4"