[27/27] lib: fwts_acpi_tables: unconstify data and don't strcpy ACPI table ids
diff mbox series

Message ID 20180815131129.24146-28-colin.king@canonical.com
State Accepted
Headers show
Series
  • [01/27] lib: fwts_framework: ensure src pointer is const
Related show

Commit Message

Colin King Aug. 15, 2018, 1:11 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

Although it seems a good idea to make the ACPI table data const, we
may need to modify it when patching pointers if we make cached copies
of the data, so unconstify it.

Also, don't strncpy ACPI table IDs, instead memcpy them as they are
not really strings with terminating zero bytes.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/include/fwts_acpi_tables.h |  2 +-
 src/lib/src/fwts_acpi_tables.c     | 24 ++++++++++++------------
 2 files changed, 13 insertions(+), 13 deletions(-)

Comments

Alex Hung Aug. 15, 2018, 6:12 p.m. UTC | #1
On 2018-08-15 06:11 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Although it seems a good idea to make the ACPI table data const, we
> may need to modify it when patching pointers if we make cached copies
> of the data, so unconstify it.
> 
> Also, don't strncpy ACPI table IDs, instead memcpy them as they are
> not really strings with terminating zero bytes.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/include/fwts_acpi_tables.h |  2 +-
>   src/lib/src/fwts_acpi_tables.c     | 24 ++++++++++++------------
>   2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
> index d307af89..24a7b78a 100644
> --- a/src/lib/include/fwts_acpi_tables.h
> +++ b/src/lib/include/fwts_acpi_tables.h
> @@ -34,7 +34,7 @@ typedef enum {
>   
>   typedef struct {
>   	char	   name[5];
> -	const void *data;
> +	void       *data;
>   	size_t	   length;
>   	uint32_t   which;
>   	uint32_t   index;
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index ccf0a7b1..b6d8fbe4 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -228,7 +228,7 @@ static void *fwts_acpi_load_table(const off_t addr)
>    */
>   static void fwts_acpi_add_table(
>   	const char *name,			/* Table Name */
> -	const void *table,			/* Table binary blob */
> +	void *table,				/* Table binary blob */
>   	const uint64_t addr,			/* Address of table */
>   	const size_t length,			/* Length of table */
>   	const fwts_acpi_table_provenance provenance)
> @@ -1006,7 +1006,7 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>   			fwts_log_error(fw, "Cannot allocate fake FACS.");
>   			return FWTS_ERROR;
>   		}
> -		strncpy(facs->signature, "FACS", 4);
> +		(void)memcpy(facs->signature, "FACS", 4);
>   		facs->length = size;
>   		facs->hardware_signature = 0xf000a200;	/* Some signature */
>   		facs->flags = 0;
> @@ -1074,13 +1074,13 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>   				if (table && fwts_acpi_table_fixable(table))
>   					rsdt->entries[j++] = (uint32_t)table->addr;
>   
> -		strncpy(rsdt->header.signature, "RSDT", 4);
> +		(void)memcpy(rsdt->header.signature, "RSDT", 4);
>   		rsdt->header.length = size;
>   		rsdt->header.revision = 1;
> -		strncpy(rsdt->header.oem_id, "FWTSID", 6);
> -		strncpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
> +		(void)memcpy(rsdt->header.oem_id, "FWTSID", 6);
> +		(void)memcpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
>   		rsdt->header.oem_revision = 1;
> -		strncpy(rsdt->header.creator_id, "FWTS", 4);
> +		(void)memcpy(rsdt->header.creator_id, "FWTS", 4);
>   		rsdt->header.creator_revision = 1;
>   		rsdt->header.checksum = 256 - fwts_checksum((uint8_t*)rsdt, size);
>   
> @@ -1111,13 +1111,13 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>   				if (table && fwts_acpi_table_fixable(table))
>   					xsdt->entries[j++] = table->addr;
>   
> -		strncpy(xsdt->header.signature, "XSDT", 4);
> +		(void)memcpy(xsdt->header.signature, "XSDT", 4);
>   		xsdt->header.length = size;
>   		xsdt->header.revision = 2;
> -		strncpy(xsdt->header.oem_id, "FWTSID", 6);
> -		strncpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
> +		(void)memcpy(xsdt->header.oem_id, "FWTSID", 6);
> +		(void)memcpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
>   		xsdt->header.oem_revision = 1;
> -		strncpy(xsdt->header.creator_id, "FWTS", 4);
> +		(void)memcpy(xsdt->header.creator_id, "FWTS", 4);
>   		xsdt->header.creator_revision = 1;
>   		xsdt->header.checksum = 256 - fwts_checksum((uint8_t*)xsdt, size);
>   
> @@ -1139,8 +1139,8 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>   			return FWTS_ERROR;
>   		}
>   
> -		strncpy(rsdp->signature, "RSD PTR ", 8);
> -		strncpy(rsdp->oem_id, "FWTSID", 6);
> +		(void)memcpy(rsdp->signature, "RSD PTR ", 8);
> +		(void)memcpy(rsdp->oem_id, "FWTSID", 6);
>   		rsdp->revision = 2;
>   		rsdp->length = sizeof(fwts_acpi_table_rsdp);
>   		rsdp->reserved[0] = 0;
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
ivanhu Aug. 16, 2018, 9:21 a.m. UTC | #2
On 08/15/2018 09:11 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Although it seems a good idea to make the ACPI table data const, we
> may need to modify it when patching pointers if we make cached copies
> of the data, so unconstify it.
>
> Also, don't strncpy ACPI table IDs, instead memcpy them as they are
> not really strings with terminating zero bytes.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/include/fwts_acpi_tables.h |  2 +-
>  src/lib/src/fwts_acpi_tables.c     | 24 ++++++++++++------------
>  2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
> index d307af89..24a7b78a 100644
> --- a/src/lib/include/fwts_acpi_tables.h
> +++ b/src/lib/include/fwts_acpi_tables.h
> @@ -34,7 +34,7 @@ typedef enum {
>  
>  typedef struct {
>  	char	   name[5];
> -	const void *data;
> +	void       *data;
>  	size_t	   length;
>  	uint32_t   which;
>  	uint32_t   index;
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index ccf0a7b1..b6d8fbe4 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -228,7 +228,7 @@ static void *fwts_acpi_load_table(const off_t addr)
>   */
>  static void fwts_acpi_add_table(
>  	const char *name,			/* Table Name */
> -	const void *table,			/* Table binary blob */
> +	void *table,				/* Table binary blob */
>  	const uint64_t addr,			/* Address of table */
>  	const size_t length,			/* Length of table */
>  	const fwts_acpi_table_provenance provenance)
> @@ -1006,7 +1006,7 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>  			fwts_log_error(fw, "Cannot allocate fake FACS.");
>  			return FWTS_ERROR;
>  		}
> -		strncpy(facs->signature, "FACS", 4);
> +		(void)memcpy(facs->signature, "FACS", 4);
>  		facs->length = size;
>  		facs->hardware_signature = 0xf000a200;	/* Some signature */
>  		facs->flags = 0;
> @@ -1074,13 +1074,13 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>  				if (table && fwts_acpi_table_fixable(table))
>  					rsdt->entries[j++] = (uint32_t)table->addr;
>  
> -		strncpy(rsdt->header.signature, "RSDT", 4);
> +		(void)memcpy(rsdt->header.signature, "RSDT", 4);
>  		rsdt->header.length = size;
>  		rsdt->header.revision = 1;
> -		strncpy(rsdt->header.oem_id, "FWTSID", 6);
> -		strncpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
> +		(void)memcpy(rsdt->header.oem_id, "FWTSID", 6);
> +		(void)memcpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
>  		rsdt->header.oem_revision = 1;
> -		strncpy(rsdt->header.creator_id, "FWTS", 4);
> +		(void)memcpy(rsdt->header.creator_id, "FWTS", 4);
>  		rsdt->header.creator_revision = 1;
>  		rsdt->header.checksum = 256 - fwts_checksum((uint8_t*)rsdt, size);
>  
> @@ -1111,13 +1111,13 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>  				if (table && fwts_acpi_table_fixable(table))
>  					xsdt->entries[j++] = table->addr;
>  
> -		strncpy(xsdt->header.signature, "XSDT", 4);
> +		(void)memcpy(xsdt->header.signature, "XSDT", 4);
>  		xsdt->header.length = size;
>  		xsdt->header.revision = 2;
> -		strncpy(xsdt->header.oem_id, "FWTSID", 6);
> -		strncpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
> +		(void)memcpy(xsdt->header.oem_id, "FWTSID", 6);
> +		(void)memcpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
>  		xsdt->header.oem_revision = 1;
> -		strncpy(xsdt->header.creator_id, "FWTS", 4);
> +		(void)memcpy(xsdt->header.creator_id, "FWTS", 4);
>  		xsdt->header.creator_revision = 1;
>  		xsdt->header.checksum = 256 - fwts_checksum((uint8_t*)xsdt, size);
>  
> @@ -1139,8 +1139,8 @@ static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
>  			return FWTS_ERROR;
>  		}
>  
> -		strncpy(rsdp->signature, "RSD PTR ", 8);
> -		strncpy(rsdp->oem_id, "FWTSID", 6);
> +		(void)memcpy(rsdp->signature, "RSD PTR ", 8);
> +		(void)memcpy(rsdp->oem_id, "FWTSID", 6);
>  		rsdp->revision = 2;
>  		rsdp->length = sizeof(fwts_acpi_table_rsdp);
>  		rsdp->reserved[0] = 0;

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch
diff mbox series

diff --git a/src/lib/include/fwts_acpi_tables.h b/src/lib/include/fwts_acpi_tables.h
index d307af89..24a7b78a 100644
--- a/src/lib/include/fwts_acpi_tables.h
+++ b/src/lib/include/fwts_acpi_tables.h
@@ -34,7 +34,7 @@  typedef enum {
 
 typedef struct {
 	char	   name[5];
-	const void *data;
+	void       *data;
 	size_t	   length;
 	uint32_t   which;
 	uint32_t   index;
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
index ccf0a7b1..b6d8fbe4 100644
--- a/src/lib/src/fwts_acpi_tables.c
+++ b/src/lib/src/fwts_acpi_tables.c
@@ -228,7 +228,7 @@  static void *fwts_acpi_load_table(const off_t addr)
  */
 static void fwts_acpi_add_table(
 	const char *name,			/* Table Name */
-	const void *table,			/* Table binary blob */
+	void *table,				/* Table binary blob */
 	const uint64_t addr,			/* Address of table */
 	const size_t length,			/* Length of table */
 	const fwts_acpi_table_provenance provenance)
@@ -1006,7 +1006,7 @@  static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
 			fwts_log_error(fw, "Cannot allocate fake FACS.");
 			return FWTS_ERROR;
 		}
-		strncpy(facs->signature, "FACS", 4);
+		(void)memcpy(facs->signature, "FACS", 4);
 		facs->length = size;
 		facs->hardware_signature = 0xf000a200;	/* Some signature */
 		facs->flags = 0;
@@ -1074,13 +1074,13 @@  static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
 				if (table && fwts_acpi_table_fixable(table))
 					rsdt->entries[j++] = (uint32_t)table->addr;
 
-		strncpy(rsdt->header.signature, "RSDT", 4);
+		(void)memcpy(rsdt->header.signature, "RSDT", 4);
 		rsdt->header.length = size;
 		rsdt->header.revision = 1;
-		strncpy(rsdt->header.oem_id, "FWTSID", 6);
-		strncpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
+		(void)memcpy(rsdt->header.oem_id, "FWTSID", 6);
+		(void)memcpy(rsdt->header.oem_tbl_id, oem_tbl_id, 8);
 		rsdt->header.oem_revision = 1;
-		strncpy(rsdt->header.creator_id, "FWTS", 4);
+		(void)memcpy(rsdt->header.creator_id, "FWTS", 4);
 		rsdt->header.creator_revision = 1;
 		rsdt->header.checksum = 256 - fwts_checksum((uint8_t*)rsdt, size);
 
@@ -1111,13 +1111,13 @@  static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
 				if (table && fwts_acpi_table_fixable(table))
 					xsdt->entries[j++] = table->addr;
 
-		strncpy(xsdt->header.signature, "XSDT", 4);
+		(void)memcpy(xsdt->header.signature, "XSDT", 4);
 		xsdt->header.length = size;
 		xsdt->header.revision = 2;
-		strncpy(xsdt->header.oem_id, "FWTSID", 6);
-		strncpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
+		(void)memcpy(xsdt->header.oem_id, "FWTSID", 6);
+		(void)memcpy(xsdt->header.oem_tbl_id, oem_tbl_id, 8);
 		xsdt->header.oem_revision = 1;
-		strncpy(xsdt->header.creator_id, "FWTS", 4);
+		(void)memcpy(xsdt->header.creator_id, "FWTS", 4);
 		xsdt->header.creator_revision = 1;
 		xsdt->header.checksum = 256 - fwts_checksum((uint8_t*)xsdt, size);
 
@@ -1139,8 +1139,8 @@  static int fwts_acpi_load_tables_fixup(fwts_framework *fw)
 			return FWTS_ERROR;
 		}
 
-		strncpy(rsdp->signature, "RSD PTR ", 8);
-		strncpy(rsdp->oem_id, "FWTSID", 6);
+		(void)memcpy(rsdp->signature, "RSD PTR ", 8);
+		(void)memcpy(rsdp->oem_id, "FWTSID", 6);
 		rsdp->revision = 2;
 		rsdp->length = sizeof(fwts_acpi_table_rsdp);
 		rsdp->reserved[0] = 0;