[PATCH] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[PATCH] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[Christopher Clark]

Add zero-padding to #defined ACPI table strings that are copied. 
Provides sufficient characters to satisfy the length required to
fully populate the destination and prevent array-bounds warnings.

Signed-off-by: Christopher Clark <christopher.clark6@baesystems.com>
---

Please add this patch to the backport list for the next minor 4.11
release.

Prior to this: gcc 8.2 objects to memcpy past bounds:

| libxl_arm_acpi.c: In function 'make_acpi_header':
| libxl_arm_acpi.c:208:5: error: 'memcpy' forming offset [5, 6] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id));
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| libxl_arm_acpi.c:209:5: error: 'memcpy' forming offset [5, 8] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID,
sizeof(h->oem_table_id));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| libxl_arm_acpi.c:211:5: error: 'memcpy' forming offset 4 is out of the
bounds [0, 3] [-Werror=array-bounds]
|      memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID,
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|             sizeof(h->asl_compiler_id));
|             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| In function 'make_acpi_rsdp.isra.4',
|     inlined from 'libxl__prepare_acpi' at libxl_arm_acpi.c:389:5:
| libxl_arm_acpi.c:193:5: error: 'memcpy' forming offset [5, 6] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id));
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 tools/libxl/libxl_arm_acpi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 636f724..eeca1de 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -48,9 +48,9 @@ extern const unsigned char dsdt_anycpu_arm[];
 _hidden
 extern const int dsdt_anycpu_arm_len;
 
-#define ACPI_OEM_ID "Xen"
-#define ACPI_OEM_TABLE_ID "ARM"
-#define ACPI_ASL_COMPILER_ID "XL"
+#define ACPI_OEM_ID "Xen\0\0"
+#define ACPI_OEM_TABLE_ID "ARM\0\0\0\0"
+#define ACPI_ASL_COMPILER_ID "XL\0"
 
 enum {
     RSDP,
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[Stefano Stabellini]

On Tue, 14 Aug 2018, Christopher Clark wrote:
> Add zero-padding to #defined ACPI table strings that are copied. 
> Provides sufficient characters to satisfy the length required to
> fully populate the destination and prevent array-bounds warnings.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@baesystems.com>
> ---
> 
> Please add this patch to the backport list for the next minor 4.11
> release.
> 
> Prior to this: gcc 8.2 objects to memcpy past bounds:
> 
> | libxl_arm_acpi.c: In function 'make_acpi_header':
> | libxl_arm_acpi.c:208:5: error: 'memcpy' forming offset [5, 6] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id));
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | libxl_arm_acpi.c:209:5: error: 'memcpy' forming offset [5, 8] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID,
> sizeof(h->oem_table_id));
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | libxl_arm_acpi.c:211:5: error: 'memcpy' forming offset 4 is out of the
> bounds [0, 3] [-Werror=array-bounds]
> |      memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID,
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> |             sizeof(h->asl_compiler_id));
> |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | In function 'make_acpi_rsdp.isra.4',
> |     inlined from 'libxl__prepare_acpi' at libxl_arm_acpi.c:389:5:
> | libxl_arm_acpi.c:193:5: error: 'memcpy' forming offset [5, 6] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id));
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
>  tools/libxl/libxl_arm_acpi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 636f724..eeca1de 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -48,9 +48,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> -#define ACPI_OEM_ID "Xen"
> -#define ACPI_OEM_TABLE_ID "ARM"
> -#define ACPI_ASL_COMPILER_ID "XL"
> +#define ACPI_OEM_ID "Xen\0\0"
> +#define ACPI_OEM_TABLE_ID "ARM\0\0\0\0"
> +#define ACPI_ASL_COMPILER_ID "XL\0"
>  
>  enum {
>      RSDP,

Thank you for the patch! These changes should fix the errors.
Alternatively, changing the three memcpy's in the following way would
also work:

  memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(ACPI_OEM_ID));

I don't have an opinion on which way is best so:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[Wei Liu]

On Tue, Aug 14, 2018 at 01:38:09PM -0700, Christopher Clark wrote:
> Add zero-padding to #defined ACPI table strings that are copied. 
> Provides sufficient characters to satisfy the length required to
> fully populate the destination and prevent array-bounds warnings.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@baesystems.com>
> ---
> 
> Please add this patch to the backport list for the next minor 4.11
> release.
> 
> Prior to this: gcc 8.2 objects to memcpy past bounds:
> 
> | libxl_arm_acpi.c: In function 'make_acpi_header':
> | libxl_arm_acpi.c:208:5: error: 'memcpy' forming offset [5, 6] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id));
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Please add BUILD_BUG_ON to check the size of oem_id and ACPI_OEM_ID are
actually the same.

> | libxl_arm_acpi.c:209:5: error: 'memcpy' forming offset [5, 8] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID,
> sizeof(h->oem_table_id));
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | libxl_arm_acpi.c:211:5: error: 'memcpy' forming offset 4 is out of the
> bounds [0, 3] [-Werror=array-bounds]
> |      memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID,
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> |             sizeof(h->asl_compiler_id));
> |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | In function 'make_acpi_rsdp.isra.4',
> |     inlined from 'libxl__prepare_acpi' at libxl_arm_acpi.c:389:5:
> | libxl_arm_acpi.c:193:5: error: 'memcpy' forming offset [5, 6] is out
> of the bounds [0, 4] [-Werror=array-bounds]
> |      memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id));
> |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
>  tools/libxl/libxl_arm_acpi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 636f724..eeca1de 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -48,9 +48,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> -#define ACPI_OEM_ID "Xen"
> -#define ACPI_OEM_TABLE_ID "ARM"
> -#define ACPI_ASL_COMPILER_ID "XL"
> +#define ACPI_OEM_ID "Xen\0\0"
> +#define ACPI_OEM_TABLE_ID "ARM\0\0\0\0"
> +#define ACPI_ASL_COMPILER_ID "XL\0"
>  
>  enum {
>      RSDP,
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[Christopher Clark]

Add zero-padding to #defined ACPI table strings that are copied.
Provides sufficient characters to satisfy the length required to
fully populate the destination and prevent array-bounds warnings.
Add BUILD_BUG_ON sizeof checks for compile-time length checking.

Signed-off-by: Christopher Clark <christopher.clark6@baesystems.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2: add BUILD_BUG_ON length checks, requested by Wei.

v1: Please add this patch to the backport list for the next minor
    4.11 release.

Prior to this: gcc 8.2 objects to memcpy past bounds:

| libxl_arm_acpi.c: In function 'make_acpi_header':
| libxl_arm_acpi.c:208:5: error: 'memcpy' forming offset [5, 6] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id));
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| libxl_arm_acpi.c:209:5: error: 'memcpy' forming offset [5, 8] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID,
sizeof(h->oem_table_id));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| libxl_arm_acpi.c:211:5: error: 'memcpy' forming offset 4 is out of the
bounds [0, 3] [-Werror=array-bounds]
|      memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID,
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|             sizeof(h->asl_compiler_id));
|             ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| In function 'make_acpi_rsdp.isra.4',
|     inlined from 'libxl__prepare_acpi' at libxl_arm_acpi.c:389:5:
| libxl_arm_acpi.c:193:5: error: 'memcpy' forming offset [5, 6] is out
of the bounds [0, 4] [-Werror=array-bounds]
|      memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id));
|      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 tools/libxl/libxl_arm_acpi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 636f724..ba874c3 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -48,9 +48,9 @@ extern const unsigned char dsdt_anycpu_arm[];
 _hidden
 extern const int dsdt_anycpu_arm_len;
 
-#define ACPI_OEM_ID "Xen"
-#define ACPI_OEM_TABLE_ID "ARM"
-#define ACPI_ASL_COMPILER_ID "XL"
+#define ACPI_OEM_ID "Xen\0\0"
+#define ACPI_OEM_TABLE_ID "ARM\0\0\0\0"
+#define ACPI_ASL_COMPILER_ID "XL\0"
 
 enum {
     RSDP,
@@ -190,6 +190,7 @@ static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
     struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset;
 
     memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
+    BUILD_BUG_ON(sizeof(ACPI_OEM_ID) != sizeof(rsdp->oem_id));
     memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id));
     rsdp->length = acpitables[RSDP].size;
     rsdp->revision = 0x02;
@@ -205,9 +206,12 @@ static void make_acpi_header(struct acpi_table_header *h, const char *sig,
     memcpy(h->signature, sig, 4);
     h->length = len;
     h->revision = rev;
+    BUILD_BUG_ON(sizeof(ACPI_OEM_ID) != sizeof(h->oem_id));
     memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id));
+    BUILD_BUG_ON(sizeof(ACPI_OEM_TABLE_ID) != sizeof(h->oem_table_id));
     memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID, sizeof(h->oem_table_id));
     h->oem_revision = 0;
+    BUILD_BUG_ON(sizeof(ACPI_ASL_COMPILER_ID) != sizeof(h->asl_compiler_id));
     memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID,
            sizeof(h->asl_compiler_id));
     h->asl_compiler_revision = 0;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2

[Wei Liu]

On Thu, Aug 16, 2018 at 01:22:41PM -0700, Christopher Clark wrote:
> Add zero-padding to #defined ACPI table strings that are copied.
> Provides sufficient characters to satisfy the length required to
> fully populate the destination and prevent array-bounds warnings.
> Add BUILD_BUG_ON sizeof checks for compile-time length checking.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@baesystems.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel