From 621024090b7729c8c698c8ab916d792846d20818 Mon Sep 17 00:00:00 2001 From: Michael Chang Date: Thu, 11 Apr 2019 17:14:05 +0800 Subject: [PATCH] hfsplus: Fix gcc9 error with -Waddress-of-packed-member The catkey->name could be unaligned since the address of 'void* record' is calculated as offset in bytes to a malloc buffer. The fix is using aligned buffer allocated by grub_malloc for holding the UTF16 string copied from catkey->name. And use that buffer as argument for grub_utf16_to_utf8 to convert to UTF8 strings. In addition, using a new copy of buffer rather than catkey->name itself for processing the endianess conversion, we can also get rid of the hunk restoring byte order of catkey->name to what it was previously. [ 59s] ../grub-core/fs/hfsplus.c: In function 'list_nodes': [ 59s] ../grub-core/fs/hfsplus.c:738:57: error: taking address of packed member of 'struct grub_hfsplus_catkey' may result in an unaligned pointer value [-Werror=address-of-packed-member] [ 59s] 738 | *grub_utf16_to_utf8 ((grub_uint8_t *) filename, catkey->name, [ 59s] | ~~~~~~^~~~~~ [ 59s] ../grub-core/fs/hfsplus.c: In function 'grub_hfsplus_label': [ 59s] ../grub-core/fs/hfsplus.c:1019:57: error: taking address of packed member of 'struct grub_hfsplus_catkey' may result in an unaligned pointer value [-Werror=address-of-packed-member] [ 59s] 1019 | *grub_utf16_to_utf8 ((grub_uint8_t *) (*label), catkey->name, [ 59s] | ~~~~~~^~~~~~ Signed-off-by: Michael Chang Reviewed-by: Daniel Kiper --- grub-core/fs/hfsplus.c | 57 +++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index 73ae95fbc..54786bb1c 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -661,6 +661,7 @@ list_nodes (void *record, void *hook_arg) char *filename; int i; struct grub_fshelp_node *node; + grub_uint16_t *keyname; struct grub_hfsplus_catfile *fileinfo; enum grub_fshelp_filetype type = GRUB_FSHELP_UNKNOWN; struct list_nodes_ctx *ctx = hook_arg; @@ -719,32 +720,34 @@ list_nodes (void *record, void *hook_arg) if (! filename) return 0; + keyname = grub_malloc (grub_be_to_cpu16 (catkey->namelen) * sizeof (*keyname)); + if (!keyname) + { + grub_free (filename); + return 0; + } + /* Make sure the byte order of the UTF16 string is correct. */ for (i = 0; i < grub_be_to_cpu16 (catkey->namelen); i++) { - catkey->name[i] = grub_be_to_cpu16 (catkey->name[i]); + keyname[i] = grub_be_to_cpu16 (catkey->name[i]); - if (catkey->name[i] == '/') - catkey->name[i] = ':'; + if (keyname[i] == '/') + keyname[i] = ':'; /* If the name is obviously invalid, skip this node. */ - if (catkey->name[i] == 0) + if (keyname[i] == 0) { + grub_free (keyname); grub_free (filename); return 0; } } - *grub_utf16_to_utf8 ((grub_uint8_t *) filename, catkey->name, + *grub_utf16_to_utf8 ((grub_uint8_t *) filename, keyname, grub_be_to_cpu16 (catkey->namelen)) = '\0'; - /* Restore the byte order to what it was previously. */ - for (i = 0; i < grub_be_to_cpu16 (catkey->namelen); i++) - { - if (catkey->name[i] == ':') - catkey->name[i] = '/'; - catkey->name[i] = grub_be_to_cpu16 (catkey->name[i]); - } + grub_free (keyname); /* hfs+ is case insensitive. */ if (! ctx->dir->data->case_sensitive) @@ -975,6 +978,7 @@ grub_hfsplus_label (grub_device_t device, char **label) grub_disk_t disk = device->disk; struct grub_hfsplus_catkey *catkey; int i, label_len; + grub_uint16_t *label_name; struct grub_hfsplus_key_internal intern; struct grub_hfsplus_btnode *node = NULL; grub_disk_addr_t ptr = 0; @@ -1003,22 +1007,41 @@ grub_hfsplus_label (grub_device_t device, char **label) grub_hfsplus_btree_recptr (&data->catalog_tree, node, ptr); label_len = grub_be_to_cpu16 (catkey->namelen); + label_name = grub_malloc (label_len * sizeof (*label_name)); + if (!label_name) + { + grub_free (node); + grub_free (data); + return grub_errno; + } + for (i = 0; i < label_len; i++) { - catkey->name[i] = grub_be_to_cpu16 (catkey->name[i]); + label_name[i] = grub_be_to_cpu16 (catkey->name[i]); /* If the name is obviously invalid, skip this node. */ - if (catkey->name[i] == 0) - return 0; + if (label_name[i] == 0) + { + grub_free (label_name); + grub_free (node); + grub_free (data); + return 0; + } } *label = grub_malloc (label_len * GRUB_MAX_UTF8_PER_UTF16 + 1); if (! *label) - return grub_errno; + { + grub_free (label_name); + grub_free (node); + grub_free (data); + return grub_errno; + } - *grub_utf16_to_utf8 ((grub_uint8_t *) (*label), catkey->name, + *grub_utf16_to_utf8 ((grub_uint8_t *) (*label), label_name, label_len) = '\0'; + grub_free (label_name); grub_free (node); grub_free (data);