Message ID | 20210623094154.421169-1-dja@axtens.net |
---|---|
State | Accepted |
Headers | show |
Series | [v2] secvar/backend: use endian-aware types in edk2.h | 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 |
On 6/23/21 3:11 PM, Daniel Axtens wrote: > Recently we had an issue where we did the following: > > uint16_t year = le32_to_cpu(timestamp->year); > > This is wrong and will break on BE. However, we didn't catch this > with sparse because there was a whole slew of warnings. > > The reason for the slew of warnings is that we didn't annotate the > types that store little-endian specific data in edk2.h. > > Provide the appropriate annotations. > > We now get a single sparse warning for the file, which correctly > identifies the issue: > > edk2-compat-process.c:374:46: warning: incorrect type in argument 1 (different base types) > edk2-compat-process.c:374:46: expected restricted leint32_t [usertype] le_val > edk2-compat-process.c:374:46: got restricted leint16_t const [usertype] year > > We do lose the signedness of efi_time->timezone, but that's probably OK: > we never use the timezone anyway and the comment above the data structure > makes the signedness pretty clear. > > Signed-off-by: Daniel Axtens <dja@axtens.net> Thanks! Merged to master as d722dc185. -Vasant
diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h index ef6d7c79e7ff..d76251482ac2 100644 --- a/libstb/secvar/backend/edk2.h +++ b/libstb/secvar/backend/edk2.h @@ -125,15 +125,15 @@ static const uuid_t EFI_CERT_RSA2048_GUID = {{ 0xe8, 0x66, 0x57, 0x3c, 0x9c, 0x2 * TimeZone: -1440 to 1440 or 2047 */ struct efi_time { - u16 year; + le16 year; u8 month; u8 day; u8 hour; u8 minute; u8 second; u8 pad1; - u32 nanosecond; - s16 timezone; + le32 nanosecond; + le16 timezone; u8 daylight; u8 pad2; }; @@ -163,15 +163,15 @@ typedef struct __packed { /// /// Total size of the signature list, including this header. /// - uint32_t SignatureListSize; + leint32_t SignatureListSize; /// /// Size of the signature header which precedes the array of signatures. /// - uint32_t SignatureHeaderSize; + leint32_t SignatureHeaderSize; /// /// Size of each signature. /// - uint32_t SignatureSize; + leint32_t SignatureSize; /// /// Header before the array of signatures. The format of this header is specified /// by the SignatureType. @@ -191,18 +191,18 @@ struct win_certificate { * The length of the entire certificate, including the length of the * header, in bytes. */ - u32 dw_length; + le32 dw_length; /* * The revision level of the WIN_CERTIFICATE structure. The current * revision level is 0x0200. */ - u16 w_revision; + le16 w_revision; /* * The certificate type. See WIN_CERT_TYPE_xxx for the UEFI certificate * types. The UEFI specification reserves the range of certificate type * values from 0x0EF0 to 0x0EFF. */ - u16 w_certificate_type; + le16 w_certificate_type; /* * The following is the actual certificate. The format of * the certificate depends on wCertificateType.
Recently we had an issue where we did the following: uint16_t year = le32_to_cpu(timestamp->year); This is wrong and will break on BE. However, we didn't catch this with sparse because there was a whole slew of warnings. The reason for the slew of warnings is that we didn't annotate the types that store little-endian specific data in edk2.h. Provide the appropriate annotations. We now get a single sparse warning for the file, which correctly identifies the issue: edk2-compat-process.c:374:46: warning: incorrect type in argument 1 (different base types) edk2-compat-process.c:374:46: expected restricted leint32_t [usertype] le_val edk2-compat-process.c:374:46: got restricted leint16_t const [usertype] year We do lose the signedness of efi_time->timezone, but that's probably OK: we never use the timezone anyway and the comment above the data structure makes the signedness pretty clear. Signed-off-by: Daniel Axtens <dja@axtens.net> --- This doesn't fix any actual bug, it just makes it easier for us to pick up endian issues in future. --- libstb/secvar/backend/edk2.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)