rhboot / fwupdate

System firmware update support for UEFI machines
99 stars 47 forks source link

Don't dereference potentially unaligned pointers into packed structs #132

Closed dankamongmen closed 4 years ago

dankamongmen commented 5 years ago

GCC 9.1.0 warns all over the place about potential unaligned pointers here, due to the packed struct update_info_s. Where we're using this struct, either extract values into intermediate safe marshals, or ensure we're accessing them through safe interfaces such as libefi's CopyMem() and CompareMem(). Also, add const to the arguments of guid_cmp().

dankamongmen commented 5 years ago

Hrmm, the build break doesn't appear due to my change, but perhaps due to an updated libefi? Yours wants unsigned char for the first arg; mine wants char; either way, this has nothing to do with my change :).

If libefi did indeed change on your testing platform, the fix is as simple as:

[grimes](0) $ git diff
diff --git a/linux/libfwup.c b/linux/libfwup.c
index 7dcb158..cf584c0 100644
--- a/linux/libfwup.c
+++ b/linux/libfwup.c
@@ -2102,8 +2102,8 @@ fwup_print_update_info(void)
                update_info *info = re->info;
                efi_guid_t guid = info->guid;
                char *id_guid = NULL;
+         unsigned char *path;
                ssize_t dp_sz;
-           char *path;

                rc = efi_guid_to_id_guid(&guid, &id_guid);
                if (rc < 0)
[grimes](0) $ 
dankamongmen commented 5 years ago

whoa, hey, @hughsie this is your project? I've got a ColorHug2 sitting on the shelf behind me :).

hughsie commented 5 years ago

whoa, hey, @hughsie this is your project?

Well, I help a bit, but really I just absorbed fwupdate into fwupd, which I'm responsible for -- see: https://blogs.gnome.org/hughsie/2018/07/02/fwupdate-is-nearly-dead-long-live-fwupd/

dankamongmen commented 5 years ago

whoa, hey, @hughsie this is your project?

Well, I help a bit, but really I just absorbed fwupdate into fwupd, which I'm responsible for -- see: https://blogs.gnome.org/hughsie/2018/07/02/fwupdate-is-nearly-dead-long-live-fwupd/

cool, well either way, the colorhug2 is a nice bit of work. i was happy to be one of the early purchasers, and always look for excuses to use it :D.

dankamongmen commented 5 years ago

@vathpela , any chance this can get merged? Even if fwupdate has been superseded, references to it remain all over the internet, and people are clearly hoping to use it. Without this, it can't be built using gcc 9 (see #129).

gardotd426 commented 4 years ago

Guess this is a dead issue/not going to be fixed?

hughsie commented 4 years ago

I don't think you want to be shipping fwupdate as a standalone project. Use fwupd!

superm1 commented 4 years ago

won't fix: see https://github.com/rhboot/fwupdate/pull/136