| Message ID | 20210620151347.122117-1-nayna@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | secvar: fix endian conversion | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (1cdde9466ab658fb5b5a53af8c4e6a8929eef698) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
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@axtens.net> Kind regards, Daniel > > Signed-off-by: Nayna Jain <nayna@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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 6/20/21 8:43 PM, Nayna Jain wrote: > unpack_timestamp() calls le32_to_cpu() for endian conversion of > uint16_t "year" value. This patch fixes the code to use le16_to_cpu(). > > Signed-off-by: Nayna Jain <nayna@linux.ibm.com> Thanks. Merged to master as 5be38b672. -Vasant
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;
unpack_timestamp() calls le32_to_cpu() for endian conversion of uint16_t "year" value. This patch fixes the code to use le16_to_cpu(). Signed-off-by: Nayna Jain <nayna@linux.ibm.com> --- libstb/secvar/backend/edk2-compat-process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)