On 08.11.24 23:29, David Laight wrote:
From: Peter Kästle
Sent: 08 November 2024 18:12
On 08.11.24 15:38, David Laight wrote:
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@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.
Good idea, but unfortunately after "git grepping" busybox repo, it seems
these kind of functions are not defined anywhere.
All I can find is those "move_from_unaligned*" functions defined inside
include/platform.h. And some "get_unaligned_be32/get_unaligned_le32",
which are obviously not endian-safe.
Could you please help me understand, where you think those functions
should come from?
...
You might need to write them :-)
But something like:
((struct { unsigned short x __attribute__((packed)); } *)bp)->x
might DTRT.
I still think this is really a good idea, but I'd see it as a different
patchset as it's an improvement and not really part of this bug fix
proposal. Furthermore other occurrences of move_from_unaligned* outside
of display() F_INT: and F_UINT: could be improved by that.
Could you please review the patch of my original mail, whether it could
be taken in use as fix of the actual problem?
best regards,
--peter;
_______________________________________________
busybox mailing list
busybox@busybox.net
https://lists.busybox.net/mailman/listinfo/busybox
