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

[Lukasz Majewski]

Hi Rommel,

> Hi All,
> 
> U-Boot 2013.07-rc3 [ELDK 5.2.1 / ELDK 5.3]
> 
> Now I've started to use the new ext4 code. I need the "ext4write"
> command. Though there seems to be several problems with the ext2/ext4
> code.

The ext4 code at u-boot (especially ext4write) needs special attention. 

It is on top of my list to refactor and clean up this code base
(especially the ext4write command). 

> 
> I am testing on an ml507 (PPC440, Big Endian).
> There are some cases where the a field is 16-bit but le32_to_cpu() is
> used. Some checks (ie eh_magic) fails to match even if I use a
> correctly ext4 formatted MMC/SD card.
> 
> Does these seem right? Or am I mistaken?

I would need to double check it (since I'm using only ext4load at my
target board currently).

> 
> -----
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 58880b4..22d4377 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -1429,7 +1429,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)
> @@ -1437,14 +1437,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))
				 ^^^^^ We had some problems with
				 unaligned access to 16 bit values
				 recently (when those start at 0x2).
				 It was observed on our ARM Exynos4
				 based target.


>  				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(block << log2_blksz, 0,
> fs->blksz, buf)) @@ -1543,17 +1543,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);
> -----
> 
> (Sorry, I can't CC anyone directly as I'm using the gmane "post"
> interface)

You can also CC ext4 u-boot's implementation authors.

> 
> All the best,
> Rommel
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Best regards,

Lukasz Majewski

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