[PATCH] libbb/dump: correct handling of unsigned on big endian

David Laight David.Laight at ACULAB.COM
Fri Nov 8 14:38:46 UTC 2024


From: Peter Kaestle
> Sent: 08 November 2024 14:02
> 
> The following commit broke dumping unsigned on big endian:
> 34751d8bf (libbb/dump: correct handling of 1-byte signed int format, 2023-05-26)
> 
> Example of the bug:
> 
> root at fct:~ >echo -ne "\xfe\xfd\xfc\xfb" > /tmp/testfile
> 
> before 34751d8bf:
> $>/tmp/busybox_1_36_1 hexdump /tmp/testfile
> 0000000 fefd fcfb
> 0000004
> 
> with 34751d8bf:
> $>busybox_1_37_0 hexdump /tmp/testfile
> 0000000 fefd00fe fcfb00fc
> 0000004
> 
> Problem originated from loading data into 32-bit "value" variable and
> then doing the memcpy of 2 or 4 bytes into value afterwards, without
> taking care of the byte order in case of big-endian cpus.
> 
> Replicating same solution used in F_INT with the union fixes the issue.

I'd suggest getting rid of move_from_unaligned16/32() and replacing
with something that just returns the value.
Then the clauses are just:
	case 2:
		value = read_unaligned_u16(bp);
		break;
and nothing will go wrong.

The code will be much smaller as well.
on x86 the above is just *(unsigned short *)bp rather than something
that (I suspect) call memcpy().

	David


> 
> Signed-off-by: Peter Kaestle <peter.kaestle at nokia.com>
> ---
>  libbb/dump.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/libbb/dump.c b/libbb/dump.c
> index b406a2428..87180dff5 100644
> --- a/libbb/dump.c
> +++ b/libbb/dump.c
> @@ -665,15 +665,23 @@ static NOINLINE void display(priv_dumper_t* dumper)
>  							conv_u(pr, bp);
>  							break;
>  						case F_UINT: {
> +							union {
> +								uint16_t val16;
> +								uint32_t val32;
> +								uint64_t val64;
> +							} u;
>  							unsigned value = (unsigned char)*bp;
> +
>  							switch (pr->bcnt) {
>  							case 1:
>  								break;
>  							case 2:
> -								move_from_unaligned16(value, bp);
> +								move_from_unaligned16(u.val16, bp);
> +								value = u.val16;
>  								break;
>  							case 4:
> -								move_from_unaligned32(value, bp);
> +								move_from_unaligned32(u.val32, bp);
> +								value = u.val32;
>  								break;
>  							/* case 8: no users yet */
>  							}
> --
> 2.35.7
> 
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> https://lists.busybox.net/mailman/listinfo/busybox

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)



More information about the busybox mailing list