Message ID | 1374396805-3008-2-git-send-email-andreas.devel@googlemail.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Hi Andreas, On Sun, Jul 21, 2013 at 2:53 AM, Andreas Bießmann < andreas.devel@googlemail.com> wrote: > From: Rommel Custodio <sessyargc+uboot@gmail.com> > > 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 <sessyargc+uboot@gmail.com> > [sent via git-send-email; rework commit message] > Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> > Reviewed-by: Simon Glass <sjg@chromium.org> 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 <sjg@chromium.org>
On Sun, 21 Jul 2013 10:53:25 +0200 Andreas Bießmann andreas.devel@googlemail.com wrote, Hi Andreas, > From: Rommel Custodio <sessyargc+uboot@gmail.com> > > 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 <sessyargc+uboot@gmail.com> > [sent via git-send-email; rework commit message] > Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com> > > --- > fs/ext4/ext4_common.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index 2776293..ff9c4ec 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -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); 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 <l.majewski@samsung.com> However, we will not run out from refactoring this code.
Hi Lukasz, On 22.07.13 08:43, Lukasz Majewski wrote: > On Sun, 21 Jul 2013 10:53:25 +0200 Andreas Bießmann > andreas.devel@googlemail.com wrote, <snip> > 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. > 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
On Sun, Jul 21, 2013 at 10:53:25AM +0200, Andreas Bie??mann wrote: > From: Rommel Custodio <sessyargc+uboot@gmail.com> > > 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 <sessyargc+uboot@gmail.com> > [sent via git-send-email; rework commit message] > Signed-off-by: Andreas Bie??mann <andreas.devel@googlemail.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Tested-by: Simon Glass <sjg@chromium.org> > Tested-by: Lukasz Majewski <l.majewski@samsung.com> 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!
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 2776293..ff9c4ec 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -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);