[Skiboot] [PATCH] secvar: fix endian conversion

Daniel Axtens dja at axtens.net
Mon Jun 21 18:53:48 AEST 2021


Hi Nayna,

> unpack_timestamp() calls le32_to_cpu() for endian conversion of
> uint16_t "year" value. This patch fixes the code to use le16_to_cpu().

I checked the rest of this file.

There are 13 *_to_cpu()s. SignatureSize, SignatureHeaderSize and
SignatureListSize are consistantly le32_to_cpu().

There's a le32_to_cpu conversion for auth_info.hdr.dw_length; that
matches the type of dw_length which is u32.

With this patch, every use of timestamp->year is converted with
le16_to_cpu().

There is only 1 cpu_to_*, which is a conversion of SECVAR_ATTRIBUTES.

[I note in passing that this will only make a difference at runtime if
the code is run as BE, I thought skiboot was run as LE these days? Maybe
I'm wrong.]

Running with `make C=1` at some point, sparse reports a number of issues
with this file:

edk2-compat-process.c:109:32: warning: incorrect type in argument 1 (different base types)
edk2-compat-process.c:109:32:    expected restricted leint32_t [usertype] le_val
edk2-compat-process.c:109:32:    got unsigned int [usertype] SignatureListSize
...
edk2-compat-process.c:402:9: warning: incorrect type in argument 1 (different base types)
edk2-compat-process.c:402:9:    expected restricted leint16_t [usertype] le_val
edk2-compat-process.c:402:9:    got unsigned short [usertype] year

I think a lot of those relate to the structure not defining members like
SignatureListSize as le32 - perhaps it would be worth seeing if any of
this should be fixed up while we're looking at endianness issues.

As far as this specific fix goes, however:
Reviewed-by: Daniel Axtens <dja at axtens.net>

Kind regards,
Daniel

>
> Signed-off-by: Nayna Jain <nayna at linux.ibm.com>
> ---
>  libstb/secvar/backend/edk2-compat-process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
> index 244f2340..037c1b49 100644
> --- a/libstb/secvar/backend/edk2-compat-process.c
> +++ b/libstb/secvar/backend/edk2-compat-process.c
> @@ -370,7 +370,7 @@ int update_timestamp(const char *key, const struct efi_time *timestamp, char *la
>  static uint64_t unpack_timestamp(const struct efi_time *timestamp)
>  {
>  	uint64_t val = 0;
> -	uint16_t year = le32_to_cpu(timestamp->year);
> +	uint16_t year = le16_to_cpu(timestamp->year);
>  
>  	/* pad1, nanosecond, timezone, daylight and pad2 are meant to be zero */
>  	val |= ((uint64_t) timestamp->pad1 & 0xFF) << 0;
> -- 
> 2.29.2
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot


More information about the Skiboot mailing list