[U-Boot] [PATCH 1/1] lib: uuid: alignment error in gen_rand_uuid()
Packed structures like struct uuid are not aligned. GCC 9.1 therefore throws an error when trying to compile gen_rand_uuid().
lib/uuid.c: In function ‘gen_rand_uuid’: lib/uuid.c:244:2: error: converting a packed ‘struct uuid’ pointer (alignment 1) to a ‘unsigned int’ pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member] 244 | unsigned int *ptr = (unsigned int *)&uuid; | ^~~~~~~~
Generate the uuid in a properly aligned buffer.
The byte order of a random number should not matter. Do not call cpu_to_be32() to change the byte order.
Reported-by: Ramon Fried rfried.dev@gmail.com Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/uuid.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/uuid.c b/lib/uuid.c index 7d7a2749b6..c18014171c 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -240,25 +240,25 @@ void uuid_bin_to_str(unsigned char *uuid_bin, char *uuid_str, int str_format) #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID) void gen_rand_uuid(unsigned char *uuid_bin) { - struct uuid uuid; - unsigned int *ptr = (unsigned int *)&uuid; + u32 ptr[4]; + struct uuid *uuid = (struct uuid *)ptr; int i;
srand(get_ticks() + rand());
/* Set all fields randomly */ - for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++) - *(ptr + i) = cpu_to_be32(rand()); + for (i = 0; i < 4; i++) + ptr[i] = rand();
- clrsetbits_be16(&uuid.time_hi_and_version, + clrsetbits_be16(&uuid->time_hi_and_version, UUID_VERSION_MASK, UUID_VERSION << UUID_VERSION_SHIFT);
- clrsetbits_8(&uuid.clock_seq_hi_and_reserved, + clrsetbits_8(&uuid->clock_seq_hi_and_reserved, UUID_VARIANT_MASK, UUID_VARIANT << UUID_VARIANT_SHIFT);
- memcpy(uuid_bin, &uuid, sizeof(struct uuid)); + memcpy(uuid_bin, uuid, 16); }
/* -- 2.22.0
Hi Heinrich,
On Sun, Jul 14, 2019 at 11:31:50PM +0200, Heinrich Schuchardt wrote:
Packed structures like struct uuid are not aligned. GCC 9.1 therefore throws an error when trying to compile gen_rand_uuid().
lib/uuid.c: In function ‘gen_rand_uuid’: lib/uuid.c:244:2: error: converting a packed ‘struct uuid’ pointer (alignment 1) to a ‘unsigned int’ pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member] 244 | unsigned int *ptr = (unsigned int *)&uuid; | ^~~~~~~~
Generate the uuid in a properly aligned buffer.
The byte order of a random number should not matter. Do not call cpu_to_be32() to change the byte order.
There is a striking resemblance between "struct uuid" [1] in U-Boot and "struct afs_uuid" [2] in Linux, with the major difference that the latter has never been defined as __packed.
Is there any compelling reason for the U-Boot struct to be packed?
[1] https://gitlab.denx.de/u-boot/u-boot/commit/4e4815feae4d3 ("lib: uuid: add functions to generate UUID version 4") -----8<----- struct uuid { unsigned int time_low; unsigned short time_mid; unsigned short time_hi_and_version; unsigned char clock_seq_hi_and_reserved; unsigned char clock_seq_low; unsigned char node[6]; } __packed; -----8<-----
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -----8<----- struct afs_uuid { __be32 time_low; /* low part of timestamp */ __be16 time_mid; /* mid part of timestamp */ __be16 time_hi_and_version; /* high part of timestamp and version */ __u8 clock_seq_hi_and_reserved; /* clock seq hi and variant */ __u8 clock_seq_low; /* clock seq low */ __u8 node[6]; /* spatially unique node ID (MAC addr) */ }; -----8<-----
On 7/15/19 10:42 AM, Eugeniu Rosca wrote:
Hi Heinrich,
On Sun, Jul 14, 2019 at 11:31:50PM +0200, Heinrich Schuchardt wrote:
Packed structures like struct uuid are not aligned. GCC 9.1 therefore throws an error when trying to compile gen_rand_uuid().
lib/uuid.c: In function ‘gen_rand_uuid’: lib/uuid.c:244:2: error: converting a packed ‘struct uuid’ pointer (alignment 1) to a ‘unsigned int’ pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member] 244 | unsigned int *ptr = (unsigned int *)&uuid; | ^~~~~~~~
Generate the uuid in a properly aligned buffer.
The byte order of a random number should not matter. Do not call cpu_to_be32() to change the byte order.
There is a striking resemblance between "struct uuid" [1] in U-Boot and "struct afs_uuid" [2] in Linux, with the major difference that the latter has never been defined as __packed.
Is there any compelling reason for the U-Boot struct to be packed?
Thanks for your analysis.
struct uuid is only used in lib/uuid.c inside gen_rand_uuid(). Hence there is no need make this structure packed. We should still remove the superfluous call to cpu_to_be32().
Best regards
Heinrich
[1] https://gitlab.denx.de/u-boot/u-boot/commit/4e4815feae4d3 ("lib: uuid: add functions to generate UUID version 4") -----8<----- struct uuid { unsigned int time_low; unsigned short time_mid; unsigned short time_hi_and_version; unsigned char clock_seq_hi_and_reserved; unsigned char clock_seq_low; unsigned char node[6]; } __packed; -----8<-----
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -----8<----- struct afs_uuid { __be32 time_low; /* low part of timestamp */ __be16 time_mid; /* mid part of timestamp */ __be16 time_hi_and_version; /* high part of timestamp and version */ __u8 clock_seq_hi_and_reserved; /* clock seq hi and variant */ __u8 clock_seq_low; /* clock seq low */ __u8 node[6]; /* spatially unique node ID (MAC addr) */ }; -----8<-----
On Sun, Jul 14, 2019 at 11:31:50PM +0200, Heinrich Schuchardt wrote:
Packed structures like struct uuid are not aligned. GCC 9.1 therefore throws an error when trying to compile gen_rand_uuid().
lib/uuid.c: In function ‘gen_rand_uuid’: lib/uuid.c:244:2: error: converting a packed ‘struct uuid’ pointer (alignment 1) to a ‘unsigned int’ pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member] 244 | unsigned int *ptr = (unsigned int *)&uuid; | ^~~~~~~~
Generate the uuid in a properly aligned buffer.
The byte order of a random number should not matter. Do not call cpu_to_be32() to change the byte order.
Reported-by: Ramon Fried rfried.dev@gmail.com Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Applied to u-boot/master, thanks!
participants (3)
-
Eugeniu Rosca -
Heinrich Schuchardt -
Tom Rini