[PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Michael Chang]

This patch set attempts to resolve the build failure in openSUSE build
service equipped with new gcc 9 compiler, which has added a new warning
flag -Waddress-of-packed-member. 

The new warning performs the check for taking the address of packed
member directly to a pointer variable with higher alignment requirement
and the outcome is risky to memory alignment fault on some architecture
when deferencing it.

Please help to review.

Thanks.

Michael Chang (8):
  cpio: fix gcc9 error address-of-packed-member
  jfs: fix gcc9 error address-of-packed-member
  hfs: fix gcc9 error address-of-packed-member
  hfsplus: fix gcc9 error address-of-packed-member
  acpi: fix gcc9 error address-of-packed-member
  usbtest: fix gcc9 error address-of-packed-member
  chainloader: fix gcc9 error address-of-packed-member
  efi: fix gcc9 error address-of-packed-member

 grub-core/commands/usbtest.c       | 13 ++++++++-
 grub-core/fs/cpio.c                |  5 ++--
 grub-core/fs/cpio_be.c             |  5 ++--
 grub-core/fs/hfsplus.c             | 57 ++++++++++++++++++++++++++------------
 grub-core/fs/jfs.c                 |  5 ++--
 grub-core/kern/efi/efi.c           | 27 ++++++++++++++++--
 grub-core/loader/efi/chainloader.c | 12 ++++++--
 include/grub/acpi.h                |  2 +-
 include/grub/hfs.h                 |  2 +-
 9 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.16.4

[PATCH 1/8] cpio: fix gcc9 error address-of-packed-member

[Michael Chang]

Change the read_number function to use void* type as its first argument
to silence the warning. The pointer is later type casted to
grub_uint16_t* and use grub_get_unaligned16 for safely deferencing it
for the value.

The solved gcc9 error like this.

[   59s] In file included from ../grub-core/fs/cpio.c:51:
[   59s] ../grub-core/fs/cpio_common.c: In function 'grub_cpio_find_file':
[   59s] ../grub-core/fs/cpio_common.c:58:31: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    58 |   data->size = read_number (hd.filesize, ARRAY_SIZE (hd.filesize));
[   59s]       |                             ~~^~~~~~~~~
[   59s] ../grub-core/fs/cpio_common.c:60:29: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    60 |     *mtime = read_number (hd.mtime, ARRAY_SIZE (hd.mtime));
[   59s]       |                           ~~^~~~~~
[   59s] ../grub-core/fs/cpio_common.c:61:28: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    61 |   modeval = read_number (hd.mode, ARRAY_SIZE (hd.mode));
[   59s]       |                          ~~^~~~~
[   59s] ../grub-core/fs/cpio_common.c:62:29: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    62 |   namesize = read_number (hd.namesize, ARRAY_SIZE (hd.namesize));
[   59s]       |                           ~~^~~~~~~~~
[   59s] In file included from ../grub-core/fs/cpio_be.c:51:
[   59s] ../grub-core/fs/cpio_common.c: In function 'grub_cpio_find_file':
[   59s] ../grub-core/fs/cpio_common.c:58:31: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    58 |   data->size = read_number (hd.filesize, ARRAY_SIZE (hd.filesize));
[   59s]       |                             ~~^~~~~~~~~
[   59s] ../grub-core/fs/cpio_common.c:60:29: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    60 |     *mtime = read_number (hd.mtime, ARRAY_SIZE (hd.mtime));
[   59s]       |                           ~~^~~~~~
[   59s] ../grub-core/fs/cpio_common.c:61:28: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    61 |   modeval = read_number (hd.mode, ARRAY_SIZE (hd.mode));
[   59s]       |                          ~~^~~~~
[   59s] ../grub-core/fs/cpio_common.c:62:29: error: taking address of packed member of 'struct head' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   59s]    62 |   namesize = read_number (hd.namesize, ARRAY_SIZE (hd.namesize));
[   59s]       |                           ~~^~~~~~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/fs/cpio.c    | 5 +++--
 grub-core/fs/cpio_be.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/grub-core/fs/cpio.c b/grub-core/fs/cpio.c
index dab5f9898..a1aa66689 100644
--- a/grub-core/fs/cpio.c
+++ b/grub-core/fs/cpio.c
@@ -38,11 +38,12 @@ struct head
 } GRUB_PACKED;
 
 static inline unsigned long long
-read_number (const grub_uint16_t *arr, grub_size_t size)
+read_number (const void *arr, grub_size_t size)
 {
   long long ret = 0;
+  const grub_uint16_t *p = (const grub_uint16_t *)arr;
   while (size--)
-    ret = (ret << 16) | grub_le_to_cpu16 (*arr++);
+    ret = (ret << 16) | grub_le_to_cpu16 (grub_get_unaligned16 (p++));
   return ret;
 }
 
diff --git a/grub-core/fs/cpio_be.c b/grub-core/fs/cpio_be.c
index 846548892..9460d0ec1 100644
--- a/grub-core/fs/cpio_be.c
+++ b/grub-core/fs/cpio_be.c
@@ -38,11 +38,12 @@ struct head
 } GRUB_PACKED;
 
 static inline unsigned long long
-read_number (const grub_uint16_t *arr, grub_size_t size)
+read_number (const void *arr, grub_size_t size)
 {
   long long ret = 0;
+  const grub_uint16_t *p = (const grub_uint16_t *)arr;
   while (size--)
-    ret = (ret << 16) | grub_be_to_cpu16 (*arr++);
+    ret = (ret << 16) | grub_be_to_cpu16 (grub_get_unaligned16 (p++));
   return ret;
 }
 
-- 
2.16.4

[PATCH 2/8] jfs: fix gcc9 error address-of-packed-member

[Michael Chang]

Change the le_to_cpu16_copy function to use void* type as its second
argument to silence the warning. The pointer is later type casted to
grub_uint16_t* and use grub_get_unaligned16 for safely deferencing it
for the value.

The solved gcc9 error like this.

[   60s] ../grub-core/fs/jfs.c: In function 'grub_jfs_getent':
[   60s] ../grub-core/fs/jfs.c:557:44: error: taking address of packed member of 'struct grub_jfs_leaf_dirent' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   60s]   557 |   le_to_cpu16_copy (filename + strpos, leaf->namepart, len < diro->data->namecomponentlen ? len
[   60s]       |                                        ~~~~^~~~~~~~~~
[   60s] ../grub-core/fs/jfs.c:570:48: error: taking address of packed member of 'struct grub_jfs_leaf_next_dirent' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   60s]   570 |  le_to_cpu16_copy (filename + strpos, next_leaf->namepart, len < 15 ? len : 15);
[   60s]       |                                       ~~~~~~~~~^~~~~~~~~~
[   60s] cc1: all warnings being treated as errors

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/fs/jfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c
index 09bc5608d..f1fe72192 100644
--- a/grub-core/fs/jfs.c
+++ b/grub-core/fs/jfs.c
@@ -499,10 +499,11 @@ grub_jfs_closedir (struct grub_jfs_diropen *diro)
 }
 
 static void
-le_to_cpu16_copy (grub_uint16_t *out, grub_uint16_t *in, grub_size_t len)
+le_to_cpu16_copy (grub_uint16_t *out, const void *in, grub_size_t len)
 {
+  const grub_uint16_t *p = (const grub_uint16_t *)in;
   while (len--)
-    *out++ = grub_le_to_cpu16 (*in++);
+    *out++ = grub_le_to_cpu16 (grub_get_unaligned16 (p++));
 }
 
 
-- 
2.16.4

[PATCH 3/8] hfs: fix gcc9 error address-of-packed-member

[Michael Chang]

Add missing packed attribute to struct grub_hfs_extent.

The solved gcc9 error like this.

[   83s] ../grub-core/fs/hfs.c: In function 'grub_hfs_iterate_records':
[   83s] ../grub-core/fs/hfs.c:699:9: error: taking address of packed member of 'struct grub_hfs_sblock' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   83s]   699 |      ? (&data->sblock.catalog_recs)
[   83s]       |        ~^~~~~~~~~~~~~~~~~~~~~~~~~~~
[   83s] ../grub-core/fs/hfs.c:700:9: error: taking address of packed member of 'struct grub_hfs_sblock' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[   83s]   700 |      : (&data->sblock.extent_recs));
[   83s]       |        ~^~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 include/grub/hfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/grub/hfs.h b/include/grub/hfs.h
index d935f5005..e27993c42 100644
--- a/include/grub/hfs.h
+++ b/include/grub/hfs.h
@@ -29,7 +29,7 @@ struct grub_hfs_extent
   /* The first physical block.  */
   grub_uint16_t first_block;
   grub_uint16_t count;
-};
+} GRUB_PACKED;
 
 /* HFS stores extents in groups of 3.  */
 typedef struct grub_hfs_extent grub_hfs_datarecord_t[3];
-- 
2.16.4

[PATCH 4/8] hfsplus: fix gcc9 error address-of-packed-member

[Michael Chang]

Use pointer to buffer returned by grub_malloc, which is supposed to be
suitably aligned for any data type, for holding the UTF16 string
converted to host by order from packed struct member catkey->name. The
aligned buffer is later used as argument to grub_utf16_to_utf8.

By using a new copy of buffer rather than catkey->name itself for
holding the endianess converted data, we can also get rid of the hunk
restoring byte order of catkey->name to what it was previously.

The solved gcc9 error like this.

[   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 <mchang@suse.com>
---
 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);
 
-- 
2.16.4

[PATCH 5/8] acpi: fix gcc9 error address-of-packed-member

[Michael Chang]

Add missing GRUB_PACKED attribute to struct grub_acpi_madt.

The solved gcc9 error like this.

[  233s] ../../grub-core/commands/lsacpi.c: In function 'disp_acpi_xsdt_table':
[  233s] ../../grub-core/commands/lsacpi.c:201:27: error: converting a packed 'struct grub_acpi_table_header' pointer (alignment 1) to a 'struct grub_acpi_madt' pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  233s]   201 |  disp_madt_table ((struct grub_acpi_madt *) t);
[  233s]       |                           ^~~~~~~~~~~~~~
[  233s] In file included from ../../grub-core/commands/lsacpi.c:23:
[  233s] ../../include/grub/acpi.h:50:8: note: defined here
[  233s]    50 | struct grub_acpi_table_header
[  233s]       |        ^~~~~~~~~~~~~~~~~~~~~~
[  233s] ../../include/grub/acpi.h:90:8: note: defined here
[  233s]    90 | struct grub_acpi_madt
[  233s]       |        ^~~~~~~~~~~~~~
[  233s] ../../grub-core/commands/lsacpi.c: In function 'disp_acpi_rsdt_table':
[  233s] ../../grub-core/commands/lsacpi.c:225:27: error: converting a packed 'struct grub_acpi_table_header' pointer (alignment 1) to a 'struct grub_acpi_madt' pointer (alignment 4) may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  233s]   225 |  disp_madt_table ((struct grub_acpi_madt *) t);
[  233s]       |                           ^~~~~~~~~~~~~~
[  233s] In file included from ../../grub-core/commands/lsacpi.c:23:
[  233s] ../../include/grub/acpi.h:50:8: note: defined here
[  233s]    50 | struct grub_acpi_table_header
[  233s]       |        ^~~~~~~~~~~~~~~~~~~~~~
[  233s] ../../include/grub/acpi.h:90:8: note: defined here
[  233s]    90 | struct grub_acpi_madt
[  233s]       |        ^~~~~~~~~~~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 include/grub/acpi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/grub/acpi.h b/include/grub/acpi.h
index 66148f684..84f49487d 100644
--- a/include/grub/acpi.h
+++ b/include/grub/acpi.h
@@ -93,7 +93,7 @@ struct grub_acpi_madt
   grub_uint32_t lapic_addr;
   grub_uint32_t flags;
   struct grub_acpi_madt_entry_header entries[0];
-};
+} GRUB_PACKED;
 
 enum
   {
-- 
2.16.4

[PATCH 6/8] usbtest: fix gcc9 error address-of-packed-member

[Michael Chang]

Copy the packed member descstrp->str to the buffer strbuf returned by
grub_malloc, which is supposed to be suitably aligned for any data type
and after we use strbuf as argument to grub_utf16_to_utf8.

The solved gcc9 error like this.

[  229s] ../../grub-core/commands/usbtest.c: In function 'grub_usb_get_string':
[  229s] ../../grub-core/commands/usbtest.c:104:58: error: taking address of packed member of 'struct grub_usb_desc_str' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  229s]   104 |   *grub_utf16_to_utf8 ((grub_uint8_t *) *string, descstrp->str,
[  229s]       |                                                  ~~~~~~~~^~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/commands/usbtest.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/grub-core/commands/usbtest.c b/grub-core/commands/usbtest.c
index 01cdca934..b42e746c0 100644
--- a/grub-core/commands/usbtest.c
+++ b/grub-core/commands/usbtest.c
@@ -69,6 +69,7 @@ grub_usb_get_string (grub_usb_device_t dev, grub_uint8_t index, int langid,
 {
   struct grub_usb_desc_str descstr;
   struct grub_usb_desc_str *descstrp;
+  grub_uint16_t *strbuf;
   grub_usb_err_t err;
 
   /* Only get the length.  */
@@ -101,8 +102,18 @@ grub_usb_get_string (grub_usb_device_t dev, grub_uint8_t index, int langid,
       return GRUB_USB_ERR_INTERNAL;
     }
 
-  *grub_utf16_to_utf8 ((grub_uint8_t *) *string, descstrp->str,
+  strbuf = grub_malloc (descstrp->length - sizeof (*descstrp));
+  if (!strbuf)
+    {
+      grub_free (*string);
+      grub_free (descstrp);
+      return GRUB_USB_ERR_INTERNAL;
+    }
+
+  grub_memcpy (strbuf, descstrp->str, descstrp->length - sizeof (*descstrp));
+  *grub_utf16_to_utf8 ((grub_uint8_t *) *string, strbuf,
 		       descstrp->length / 2 - 1) = 0;
+  grub_free (strbuf);
   grub_free (descstrp);
 
   return GRUB_USB_ERR_NONE;
-- 
2.16.4

[PATCH 7/8] chainloader: fix gcc9 error address-of-packed-member

[Michael Chang]

Copy the packed member fp->path_name to the buffer path_name returned by
grub_malloc, which is supposed to be suitably aligned for any data type
and use path_name as argument to grub_utf8_to_utf16. After the function
grub_utf8_to_utf16 returns, copy the content of path_name back to
fp->path_name.

The solved gcc9 error like this.

[  243s] ../../grub-core/loader/efi/chainloader.c: In function 'copy_file_path':
[  243s] ../../grub-core/loader/efi/chainloader.c:136:32: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  243s]   136 |   size = grub_utf8_to_utf16 (fp->path_name, len * GRUB_MAX_UTF16_PER_UTF8,
[  243s]       |                              ~~^~~~~~~~~~~
[  243s] ../../grub-core/loader/efi/chainloader.c:138:12: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  243s]   138 |   for (p = fp->path_name; p < fp->path_name + size; p++)
[  243s]       |            ^~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/loader/efi/chainloader.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index f706b1ac3..cd92ea3f2 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -110,21 +110,27 @@ static void
 copy_file_path (grub_efi_file_path_device_path_t *fp,
 		const char *str, grub_efi_uint16_t len)
 {
-  grub_efi_char16_t *p;
+  grub_efi_char16_t *p, *path_name;
   grub_efi_uint16_t size;
 
   fp->header.type = GRUB_EFI_MEDIA_DEVICE_PATH_TYPE;
   fp->header.subtype = GRUB_EFI_FILE_PATH_DEVICE_PATH_SUBTYPE;
 
-  size = grub_utf8_to_utf16 (fp->path_name, len * GRUB_MAX_UTF16_PER_UTF8,
+  path_name = grub_malloc (len * GRUB_MAX_UTF16_PER_UTF8 * sizeof (*path_name));
+  if (!path_name)
+    return;
+
+  size = grub_utf8_to_utf16 (path_name, len * GRUB_MAX_UTF16_PER_UTF8,
 			     (const grub_uint8_t *) str, len, 0);
-  for (p = fp->path_name; p < fp->path_name + size; p++)
+  for (p = path_name; p < path_name + size; p++)
     if (*p == '/')
       *p = '\\';
 
+  grub_memcpy (fp->path_name, path_name, size * sizeof (*fp->path_name));
   /* File Path is NULL terminated */
   fp->path_name[size++] = '\0';
   fp->header.length = size * sizeof (grub_efi_char16_t) + sizeof (*fp);
+  grub_free (path_name);
 }
 
 static grub_efi_device_path_t *
-- 
2.16.4

[PATCH 8/8] efi: fix gcc9 error address-of-packed-member

[Michael Chang]

Copy the packed member to the buffer returned by grub_malloc, which is
supposed to be suitably aligned for any data type, then use it as
argument to grub_utf16_to_utf8.

The solved gcc9 error like this.

[  255s] ../../grub-core/kern/efi/efi.c: In function 'grub_efi_get_filename':
[  255s] ../../grub-core/kern/efi/efi.c:410:60: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  255s]   410 |    p = (char *) grub_utf16_to_utf8 ((unsigned char *) p, fp->path_name, len);
[  255s]       |                                                          ~~^~~~~~~~~~~
[  255s] ../../grub-core/kern/efi/efi.c: In function 'grub_efi_print_device_path':
[  255s] ../../grub-core/kern/efi/efi.c:900:33: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  255s]   900 |     *grub_utf16_to_utf8 (buf, fp->path_name,
[  255s]       |                               ~~^~~~~~~~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
---
 grub-core/kern/efi/efi.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 708581fcb..62cd9829e 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -369,6 +369,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
 	{
 	  grub_efi_file_path_device_path_t *fp;
 	  grub_efi_uint16_t len;
+	  grub_efi_char16_t *dup_name;
 
 	  *p++ = '/';
 
@@ -379,7 +380,16 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
 	  while (len > 0 && fp->path_name[len - 1] == 0)
 	    len--;
 
-	  p = (char *) grub_utf16_to_utf8 ((unsigned char *) p, fp->path_name, len);
+	  dup_name = grub_malloc (len * sizeof (*dup_name));
+	  if (!dup_name)
+	    {
+	      grub_free (name);
+	      return NULL;
+	    }
+	  p = (char *) grub_utf16_to_utf8 ((unsigned char *) p,
+					    grub_memcpy (dup_name, fp->path_name, len * sizeof (*dup_name)),
+					    len);
+	  grub_free (dup_name);
 	}
 
       dp = GRUB_EFI_NEXT_DEVICE_PATH (dp);
@@ -809,9 +819,20 @@ grub_efi_print_device_path (grub_efi_device_path_t *dp)
 		fp = (grub_efi_file_path_device_path_t *) dp;
 		buf = grub_malloc ((len - 4) * 2 + 1);
 		if (buf)
-		  *grub_utf16_to_utf8 (buf, fp->path_name,
+		  {
+		    grub_efi_char16_t *dup_name = grub_malloc (len - 4);
+		    if (!dup_name)
+		      {
+			grub_errno = GRUB_ERR_NONE;
+			grub_printf ("/File((null))");
+			grub_free (buf);
+			break;
+		      }
+		    *grub_utf16_to_utf8 (buf, grub_memcpy (dup_name, fp->path_name, len - 4),
 				       (len - 4) / sizeof (grub_efi_char16_t))
-		    = '\0';
+		      = '\0';
+		    grub_free (dup_name);
+		  }
 		else
 		  grub_errno = GRUB_ERR_NONE;
 		grub_printf ("/File(%s)", buf);
-- 
2.16.4

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Vladimir 'phcoder' Serbinenko]

On Tue, Apr 9, 2019 at 8:48 PM Michael Chang <mchang@suse.com> wrote:
>
> This patch set attempts to resolve the build failure in openSUSE build
> service equipped with new gcc 9 compiler, which has added a new warning
> flag -Waddress-of-packed-member.
>
> The new warning performs the check for taking the address of packed
> member directly to a pointer variable with higher alignment requirement
> and the outcome is risky to memory alignment fault on some architecture
> when deferencing it.
The problem here is that there is no way to tell the compiler to
create a structure without padding but a pointer to which is aligned.
This is very common in GRUB. Consider a struct like following:
grub_uint16_t a;
grub_uint32_t b;
grub_uint16_t c[40];

This needs packed attribute as otherwise compiler will insert a
padding before b. Yet c is guaranteed to be aligned if the pointer to
the structure is aligned.
>
> Please help to review.
>
> Thanks.
>
> Michael Chang (8):
>   cpio: fix gcc9 error address-of-packed-member
>   jfs: fix gcc9 error address-of-packed-member
>   hfs: fix gcc9 error address-of-packed-member
>   hfsplus: fix gcc9 error address-of-packed-member
>   acpi: fix gcc9 error address-of-packed-member
>   usbtest: fix gcc9 error address-of-packed-member
>   chainloader: fix gcc9 error address-of-packed-member
>   efi: fix gcc9 error address-of-packed-member
>
>  grub-core/commands/usbtest.c       | 13 ++++++++-
>  grub-core/fs/cpio.c                |  5 ++--
>  grub-core/fs/cpio_be.c             |  5 ++--
>  grub-core/fs/hfsplus.c             | 57 ++++++++++++++++++++++++++------------
>  grub-core/fs/jfs.c                 |  5 ++--
>  grub-core/kern/efi/efi.c           | 27 ++++++++++++++++--
>  grub-core/loader/efi/chainloader.c | 12 ++++++--
>  include/grub/acpi.h                |  2 +-
>  include/grub/hfs.h                 |  2 +-
>  9 files changed, 96 insertions(+), 32 deletions(-)
>
> --
> 2.16.4
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Michael Chang]

On Tue, Apr 09, 2019 at 08:57:25PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> On Tue, Apr 9, 2019 at 8:48 PM Michael Chang <mchang@suse.com> wrote:
> >
> > This patch set attempts to resolve the build failure in openSUSE build
> > service equipped with new gcc 9 compiler, which has added a new warning
> > flag -Waddress-of-packed-member.
> >
> > The new warning performs the check for taking the address of packed
> > member directly to a pointer variable with higher alignment requirement
> > and the outcome is risky to memory alignment fault on some architecture
> > when deferencing it.
> The problem here is that there is no way to tell the compiler to
> create a structure without padding but a pointer to which is aligned.
> This is very common in GRUB. Consider a struct like following:
> grub_uint16_t a;
> grub_uint32_t b;
> grub_uint16_t c[40];
> 
> This needs packed attribute as otherwise compiler will insert a
> padding before b. Yet c is guaranteed to be aligned if the pointer to
> the structure is aligned.

But sometimes it is not easy to know as the packed structure could be
embedded to another and also it could map to a specific portion in
memory read from a file, with no guarantee where will be started (for eg
via searching occurrence of specific magic number ..)

Thanks,
Michael

> >
> > Please help to review.
> >
> > Thanks.
> >
> > Michael Chang (8):
> >   cpio: fix gcc9 error address-of-packed-member
> >   jfs: fix gcc9 error address-of-packed-member
> >   hfs: fix gcc9 error address-of-packed-member
> >   hfsplus: fix gcc9 error address-of-packed-member
> >   acpi: fix gcc9 error address-of-packed-member
> >   usbtest: fix gcc9 error address-of-packed-member
> >   chainloader: fix gcc9 error address-of-packed-member
> >   efi: fix gcc9 error address-of-packed-member
> >
> >  grub-core/commands/usbtest.c       | 13 ++++++++-
> >  grub-core/fs/cpio.c                |  5 ++--
> >  grub-core/fs/cpio_be.c             |  5 ++--
> >  grub-core/fs/hfsplus.c             | 57 ++++++++++++++++++++++++++------------
> >  grub-core/fs/jfs.c                 |  5 ++--
> >  grub-core/kern/efi/efi.c           | 27 ++++++++++++++++--
> >  grub-core/loader/efi/chainloader.c | 12 ++++++--
> >  include/grub/acpi.h                |  2 +-
> >  include/grub/hfs.h                 |  2 +-
> >  9 files changed, 96 insertions(+), 32 deletions(-)
> >
> > --
> > 2.16.4
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
> -- 
> Regards
> Vladimir 'phcoder' Serbinenko
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Vladimir 'phcoder' Serbinenko]

On Tue, Apr 9, 2019 at 9:40 PM Michael Chang <mchang@suse.com> wrote:
>
> On Tue, Apr 09, 2019 at 08:57:25PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> > On Tue, Apr 9, 2019 at 8:48 PM Michael Chang <mchang@suse.com> wrote:
> > >
> > > This patch set attempts to resolve the build failure in openSUSE build
> > > service equipped with new gcc 9 compiler, which has added a new warning
> > > flag -Waddress-of-packed-member.
> > >
> > > The new warning performs the check for taking the address of packed
> > > member directly to a pointer variable with higher alignment requirement
> > > and the outcome is risky to memory alignment fault on some architecture
> > > when deferencing it.
> > The problem here is that there is no way to tell the compiler to
> > create a structure without padding but a pointer to which is aligned.
> > This is very common in GRUB. Consider a struct like following:
> > grub_uint16_t a;
> > grub_uint32_t b;
> > grub_uint16_t c[40];
> >
> > This needs packed attribute as otherwise compiler will insert a
> > padding before b. Yet c is guaranteed to be aligned if the pointer to
> > the structure is aligned.
>
> But sometimes it is not easy to know as the packed structure could be
> embedded to another and also it could map to a specific portion in
> memory read from a file, with no guarantee where will be started (for eg
> via searching occurrence of specific magic number ..)
Yes. It's not easy. But that's why just following warnings isn't
always the good way to go [without going into specific details for
particular patches].
>
> Thanks,
> Michael
>
> > >
> > > Please help to review.
> > >
> > > Thanks.
> > >
> > > Michael Chang (8):
> > >   cpio: fix gcc9 error address-of-packed-member
> > >   jfs: fix gcc9 error address-of-packed-member
> > >   hfs: fix gcc9 error address-of-packed-member
> > >   hfsplus: fix gcc9 error address-of-packed-member
> > >   acpi: fix gcc9 error address-of-packed-member
> > >   usbtest: fix gcc9 error address-of-packed-member
> > >   chainloader: fix gcc9 error address-of-packed-member
> > >   efi: fix gcc9 error address-of-packed-member
> > >
> > >  grub-core/commands/usbtest.c       | 13 ++++++++-
> > >  grub-core/fs/cpio.c                |  5 ++--
> > >  grub-core/fs/cpio_be.c             |  5 ++--
> > >  grub-core/fs/hfsplus.c             | 57 ++++++++++++++++++++++++++------------
> > >  grub-core/fs/jfs.c                 |  5 ++--
> > >  grub-core/kern/efi/efi.c           | 27 ++++++++++++++++--
> > >  grub-core/loader/efi/chainloader.c | 12 ++++++--
> > >  include/grub/acpi.h                |  2 +-
> > >  include/grub/hfs.h                 |  2 +-
> > >  9 files changed, 96 insertions(+), 32 deletions(-)
> > >
> > > --
> > > 2.16.4
> > >
> > >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> >
> > --
> > Regards
> > Vladimir 'phcoder' Serbinenko
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH 0/8] fix gcc9 build with -Werror=address-of-packed-member

[Michael Chang]

On Tue, Apr 09, 2019 at 10:15:33PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> On Tue, Apr 9, 2019 at 9:40 PM Michael Chang <mchang@suse.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 08:57:25PM +1000, Vladimir 'phcoder' Serbinenko wrote:
> > > On Tue, Apr 9, 2019 at 8:48 PM Michael Chang <mchang@suse.com> wrote:
> > > >
> > > > This patch set attempts to resolve the build failure in openSUSE build
> > > > service equipped with new gcc 9 compiler, which has added a new warning
> > > > flag -Waddress-of-packed-member.
> > > >
> > > > The new warning performs the check for taking the address of packed
> > > > member directly to a pointer variable with higher alignment requirement
> > > > and the outcome is risky to memory alignment fault on some architecture
> > > > when deferencing it.
> > > The problem here is that there is no way to tell the compiler to
> > > create a structure without padding but a pointer to which is aligned.
> > > This is very common in GRUB. Consider a struct like following:
> > > grub_uint16_t a;
> > > grub_uint32_t b;
> > > grub_uint16_t c[40];
> > >
> > > This needs packed attribute as otherwise compiler will insert a
> > > padding before b. Yet c is guaranteed to be aligned if the pointer to
> > > the structure is aligned.
> >
> > But sometimes it is not easy to know as the packed structure could be
> > embedded to another and also it could map to a specific portion in
> > memory read from a file, with no guarantee where will be started (for eg
> > via searching occurrence of specific magic number ..)
> Yes. It's not easy. But that's why just following warnings isn't
> always the good way to go [without going into specific details for
> particular patches].

OK. Point taken. I'll go through the pathes again and ignore the
warnings if the diagnostic is found to be false positive.

Thanks,
Michael

> >
> > Thanks,
> > Michael
> >
> > > >
> > > > Please help to review.
> > > >
> > > > Thanks.
> > > >
> > > > Michael Chang (8):
> > > >   cpio: fix gcc9 error address-of-packed-member
> > > >   jfs: fix gcc9 error address-of-packed-member
> > > >   hfs: fix gcc9 error address-of-packed-member
> > > >   hfsplus: fix gcc9 error address-of-packed-member
> > > >   acpi: fix gcc9 error address-of-packed-member
> > > >   usbtest: fix gcc9 error address-of-packed-member
> > > >   chainloader: fix gcc9 error address-of-packed-member
> > > >   efi: fix gcc9 error address-of-packed-member
> > > >
> > > >  grub-core/commands/usbtest.c       | 13 ++++++++-
> > > >  grub-core/fs/cpio.c                |  5 ++--
> > > >  grub-core/fs/cpio_be.c             |  5 ++--
> > > >  grub-core/fs/hfsplus.c             | 57 ++++++++++++++++++++++++++------------
> > > >  grub-core/fs/jfs.c                 |  5 ++--
> > > >  grub-core/kern/efi/efi.c           | 27 ++++++++++++++++--
> > > >  grub-core/loader/efi/chainloader.c | 12 ++++++--
> > > >  include/grub/acpi.h                |  2 +-
> > > >  include/grub/hfs.h                 |  2 +-
> > > >  9 files changed, 96 insertions(+), 32 deletions(-)
> > > >
> > > > --
> > > > 2.16.4
> > > >
> > > >
> > > > _______________________________________________
> > > > Grub-devel mailing list
> > > > Grub-devel@gnu.org
> > > > https://lists.gnu.org/mailman/listinfo/grub-devel
> > >
> > >
> > >
> > > --
> > > Regards
> > > Vladimir 'phcoder' Serbinenko
> > >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> > >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
> -- 
> Regards
> Vladimir 'phcoder' Serbinenko
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel