Re: [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF

[Jan Kara]

On Mon 09-10-17 10:04:52, Steve Magnani wrote:
> Large (> 1 TiB) UDF filesystems appear subject to several problems when
> mounted on 64-bit systems:
> 
> * readdir() can fail on a directory containing File Identifiers residing
>   above 0x7FFFFFFF. This manifests as a 'ls' command failing with EIO.
> 
> * FIBMAP on a file block located above 0x7FFFFFFF can return a negative
>   value. The low 32 bits are correct, but applications that don't mask the
>   high 32 bits of the result can perform incorrectly.
> 
> * Unsigned values > 0x7FFFFFFF are output as negative numbers in some
>   driver printks, e.g.:
>   Partition (0 type 1511) starts at physical 460, block length -1779968542
>  
> Take care to use "%u" when printing unsigned values and to use unsigned
> types to store UDF block addresses.
> 
> Signed-off-by: Steven J. Magnani <steve@xxxxxxxxxxxxxxx>

Thanks for looking into this and for the patch! However the patch seems to
be mixing two changes into one which I'd prefer to be separate patches:

1) Changes so that physical block numbers are stored in uint32_t (and
accompanying format string changes). Also when doing this, could you please
create a dedicated type like

typedef uint32_t udf_pblk_t;

and use it instead of uint32_t? That way it would be cleaner what's going
on. Thanks!

2) Changes fixing signedness in various format strings for various types -
put these in a separare patch please.

> --- a/fs/udf/balloc.c	(revision 26779)
> +++ b/fs/udf/balloc.c	(working copy)
...
> @@ -151,7 +151,7 @@
>  		bh = bitmap->s_block_bitmap[bitmap_nr];
>  		for (i = 0; i < count; i++) {
>  			if (udf_set_bit(bit + i, bh->b_data)) {
> -				udf_debug("bit %ld already set\n", bit + i);
> +				udf_debug("bit %lu already set\n", bit + i);

This change looks wrong - bit and i are signed. However they are ints, not
longs, so that should indeed be fixed.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR