ext4 fixes for BE machine

PATCH New Andreas Bießmann 6 participants 8 messages Jul 21, 2013 – Jul 22, 2013 ext4 file system

Share & tools

Andreas Bießmann

author

2013-07-21 08:53 UTC #0 lore raw

cover ext4 fixes for BE machine

I just copied the patch provided by Rommel and tested reading an ext4 on avr32 machine. List content and read data from it worked, but I can't say anything about ext4write which Rommel says to fix with that patch.

@sjg: We should add Series-notes switch to avoid cover letter for single patch submission ...

Rommel Custodio (1):
ext4fs: le32_to_cpu() used on a 16-bit field

 fs/ext4/ext4_common.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Signature · 1 line

1.7.10.4

Andreas Bießmann

author

2013-07-21 08:53 UTC #1 lore raw

PATCH 1/1 ext4fs: le32_to_cpu() used on a 16-bit field

From: Rommel Custodio <[email protected]>

Fix reading ext4_extent_header struture on BE machines.
Some 16 bit fields where converted to 32 bit fields, due to the byte swap on BE machines the containing value was corrupted. Therefore reading ext4 filesystems on BE machines where broken before.

Signed-off-by Rommel Custodio <[email protected]>

[sent via git-send-email; rework commit message]

Signed-off-by Andreas Bie?mann <[email protected]>

---
 fs/ext4/ext4_common.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

fs/ext4/ext4_common.c +6 -6

index 2776293..ff9c4ec 100644
@@ -1432,7 +1432,7 @@ static struct ext4_extent_header *ext4fs_get_extent_block
 	while (1) {
 		index = (struct ext4_extent_idx *)(ext_block + 1);
 
-		if (le32_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
+		if (le16_to_cpu(ext_block->eh_magic) != EXT4_EXT_MAGIC)
 			return 0;
 
 		if (ext_block->eh_depth == 0)

@@ -1440,14 +1440,14 @@ static struct ext4_extent_header *ext4fs_get_extent_block
 		i = -1;
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock > le32_to_cpu(index[i].ei_block));
 
 		if (--i < 0)
 			return 0;
 
-		block = le32_to_cpu(index[i].ei_leaf_hi);
+		block = le16_to_cpu(index[i].ei_leaf_hi);
 		block = (block << 32) + le32_to_cpu(index[i].ei_leaf_lo);
 
 		if (ext4fs_devread((lbaint_t)block << log2_blksz, 0, fs->blksz,

@@ -1548,17 +1548,17 @@ long int read_allocated_block(struct ext2_inode *inode, int fileblock)
 
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock >= le32_to_cpu(extent[i].ee_block));
 		if (--i >= 0) {
 			fileblock -= le32_to_cpu(extent[i].ee_block);
-			if (fileblock >= le32_to_cpu(extent[i].ee_len)) {
+			if (fileblock >= le16_to_cpu(extent[i].ee_len)) {
 				free(buf);
 				return 0;
 			}
 
-			start = le32_to_cpu(extent[i].ee_start_hi);
+			start = le16_to_cpu(extent[i].ee_start_hi);
 			start = (start << 32) +
 					le32_to_cpu(extent[i].ee_start_lo);
 			free(buf);

Signature · 1 line

1.7.10.4

Simon Glass

2013-07-21 18:13 UTC #2 lore raw

PATCH 1/1 ext4fs: le32_to_cpu() used on a 16-bit field

Hi Andreas,

Quoted 9 lines · Andreas Bie?mann

From: Rommel Custodio <[email protected]>
Fix reading ext4_extent_header struture on BE machines.
Some 16 bit fields where converted to 32 bit fields, due to the byte swap on BE
machines the containing value was corrupted. Therefore reading ext4 filesystems
on BE machines where broken before.

Quoted 4 lines

Signed-off-by Rommel Custodio <[email protected]>
[sent via git-send-email; rework commit message]
Signed-off-by Andreas Bie?mann <[email protected]>

Reviewed-by Simon Glass <[email protected]>

I tested it on sandbox with the sandbox block patch (not yet merged). It worked fine with my simple test (loading a kernel and a 70MB executable) with and without this patch unfortunately, but anyway:

Tested-by Simon Glass <[email protected]>

Lukasz Majewski

2013-07-22 06:43 UTC #3 lore raw

PATCH 1/1 ext4fs: le32_to_cpu() used on a 16-bit field

In reply to Andreas Bießmann

On Sun, 21 Jul 2013 10:53:25 +0200 Andreas Bie?mann andreas.devel at googlemail.com wrote,

Hi Andreas,

Quoted 60 lines

From: Rommel Custodio <[email protected]>
Fix reading ext4_extent_header struture on BE machines. Some 16 bit fields where converted to 32 bit fields, due to the byte swap on BE machines the containing value was corrupted. Therefore reading ext4 filesystems on BE machines where broken before.
Signed-off-by Rommel Custodio <[email protected]>
[sent via git-send-email; rework commit message]
Signed-off-by Andreas Bie?mann <[email protected]>
---
 fs/ext4/ext4_common.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
fs/ext4/ext4_common.c +1 -1
index 2776293..ff9c4ec 100644
@@ -1432,7 +1432,7 @@ static struct ext4_extent_header
 *ext4fs_get_extent_block while (1) {
 		index = (struct ext4_extent_idx *)(ext_block + 1);
 
-		if (le32_to_cpu(ext_block->eh_magic) !=
 EXT4_EXT_MAGIC)
+		if (le16_to_cpu(ext_block->eh_magic) !=
 EXT4_EXT_MAGIC) return 0;
 
fs/ext4/ext4_common.c +2 -2
 		if (ext_block->eh_depth == 0)
@@ -1440,14 +1440,14 @@ static struct ext4_extent_header
 *ext4fs_get_extent_block i = -1;
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock > le32_to_cpu(index[i].ei_block));
 
 		if (--i < 0)
 			return 0;
 
-		block = le32_to_cpu(index[i].ei_leaf_hi);
+		block = le16_to_cpu(index[i].ei_leaf_hi);
 		block = (block << 32) +
 le32_to_cpu(index[i].ei_leaf_lo); 
 		if (ext4fs_devread((lbaint_t)block << log2_blksz, 0,
fs->blksz, @@ -1548,17 +1548,17 @@ long int read_allocated_block(struct ext2_inode *inode, int fileblock)
fs/ext4/ext4_common.c +1 -1
 		do {
 			i++;
-			if (i >= le32_to_cpu(ext_block->eh_entries))
+			if (i >= le16_to_cpu(ext_block->eh_entries))
 				break;
 		} while (fileblock >=
le32_to_cpu(extent[i].ee_block)); if (--i >= 0) {
fs/ext4/ext4_common.c +0 -1
 			fileblock -= le32_to_cpu(extent[i].ee_block);
-			if (fileblock >=
le32_to_cpu(extent[i].ee_len)) {
fs/ext4/ext4_common.c +1 -0
+			if (fileblock >=
le16_to_cpu(extent[i].ee_len)) { free(buf);

fs/ext4/ext4_common.c long int read_allocated_block(struct ext2_inode *inode, int fileblock) · line 1564

Quoted 8 lines

fs/ext4/ext4_common.c +1 -1

 				return 0;
 			}
 
-			start = le32_to_cpu(extent[i].ee_start_hi);
+			start = le16_to_cpu(extent[i].ee_start_hi);
 			start = (start << 32) +
 					le32_to_cpu(extent[i].ee_start_lo);
 			free(buf);

I've tested this patch at LE Samsung Trats board. The code worked as before (ext4ls, ext4load, ext4write) - also taking into account limitation of our platform.

Tested-by Lukasz Majewski <[email protected]>

However, we will not run out from refactoring this code.

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Andreas Bießmann

2013-07-22 07:07 UTC #4 lore raw

PATCH 1/1 ext4fs: le32_to_cpu() used on a 16-bit field

Hi Lukasz,

Quoted 2 lines · Lukasz Majewski

On Sun, 21 Jul 2013 10:53:25 +0200 Andreas Bie?mann andreas.devel at googlemail.com wrote,

<snip>

Quoted 3 lines

I've tested this patch at LE Samsung Trats board. The code worked as before (ext4ls, ext4load, ext4write) - also taking into account limitation of our platform.

I'm glad to hear that your proposed unaligned access problems not apply.

Quoted 1 line

However, we will not run out from refactoring this code.

I'm fine with that for later releases, this fix should go into 2013.07.

Best regards,

Andreas Bie?mann

Tom Rini

2013-07-22 14:11 UTC #5 lore raw

[U-Boot] ext4fs: le32_to_cpu() used on a 16-bit field

In reply to Andreas Bießmann

Quoted 7 lines · Andreas Bie??mann

From: Rommel Custodio <[email protected]>
Fix reading ext4_extent_header struture on BE machines. Some 16 bit fields where converted to 32 bit fields, due to the byte swap on BE machines the containing value was corrupted. Therefore reading ext4 filesystems on BE machines where broken before.

Quoted 6 lines

Signed-off-by Rommel Custodio <[email protected]>
[sent via git-send-email; rework commit message]
Signed-off-by Andreas Bie??mann <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Tested-by Simon Glass <[email protected]>
Tested-by Lukasz Majewski <[email protected]>

So I gave the changes a review myself based on how the kernel accesses these fields and agree the changes are correct. I also did a quick test on beaglebone black and was able to read (and confirmed crc32) a few files correctly still. Applied to u-boot/master with a re-wrapping of the commit message, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130722/3f2ba1b3/attachment.pgp>

Simon Glass

2013-07-21 18:20 UTC #6 lore raw

cover ext4 fixes for BE machine

In reply to Andreas Bießmann

Hi Andreas,

Quoted 9 lines · Andreas Bie?mann

I just copied the patch provided by Rommel and tested reading an ext4 on avr32
machine. List content and read data from it worked, but I can't say anything
about ext4write which Rommel says to fix with that patch.
@sjg: We should add Series-notes switch to avoid cover letter for single patch submission ...

Yes agreed, There is Series-notes, but it's not quite what you want, I think.

Regards,
Simon

Rommel G Custodio

2013-07-21 20:09 UTC #7 lore raw

cover ext4 fixes for BE machine

In reply to Andreas Bießmann

Dear Andreas Bie?mann

Quoted 3 lines · Andreas Bie?mann

I just copied the patch provided by Rommel and tested reading an ext4 on avr32 machine. List content and read data from it worked, but I can't say anything about ext4write which Rommel says to fix with that patch.

Thanks for update.

This patch doesn't fix ext4write in BE. It just correctly accesses the ext3/ext4 formatted partition so that ext4ls/ext4load can work correctly.

I'm still checking. I'll submit a patch when I got it working.

Thank you.

Quoted 17 lines

@sjg: We should add Series-notes switch to avoid cover letter for single patch submission ...
Rommel Custodio (1):
ext4fs: le32_to_cpu() used on a 16-bit field
 fs/ext4/ext4_common.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Signature · 6 lines
1.7.10.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot