diff mbox series

tcgbios: Fix warnings

Message ID 20210630031614.2906767-1-aik@ozlabs.ru
State Accepted
Headers show
Series tcgbios: Fix warnings | expand

Commit Message

Alexey Kardashevskiy June 30, 2021, 3:16 a.m. UTC
This fixes gcc warnings from -Waddress-of-packed-member and -Wzero-length-bounds.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

---

tpm_drivers.c: In function ‘spapr_send_crq_and_wait’:
tpm_drivers.c:153:2: warning: converting a packed ‘struct crq’ pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} pointer alignment 8) may result in an unaligned pointer value [-Waddress-of-packed-member]
  153 |  rc = hv_send_crq(unit, (uint64_t *)crq);
      |  ^~
tpm_drivers.c:34:8: note: defined here
   34 | struct crq {
      |        ^~~
tpm_drivers.c: In function ‘spapr_vtpm_senddata’:
tpm_drivers.c:346:2: warning: converting a packed ‘struct crq’ pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} pointer (alignment 8) may result in an unaligned pointer value [-Waddress-of-packed-member]
  346 |  rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
      |  ^~
tpm_drivers.c:34:8: note: defined here
   34 | struct crq {
      |        ^~~
        [CC]    common-libs
        [CC]    common-libs
tcgbios.c: In function ‘tpm20_write_EfiSpecIdEventStruct’:
tcgbios.c:708:24: warning: array subscript ‘numAlgs’ is outside the bounds of an interior zero-length array ‘struct TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
  708 |   event.hdr.digestSizes[numAlgs].algorithmId =
      |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
In file included from tpm_drivers.h:20,
                 from tcgbios.c:27:
tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
   92 |  } digestSizes[0];
      |    ^~~~~~~~~~~
tcgbios.c:710:24: warning: array subscript ‘numAlgs’ is outside the bounds of an interior zero-length array ‘struct TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
  710 |   event.hdr.digestSizes[numAlgs].digestSize = cpu_to_log16(hsize);
      |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
In file included from tpm_drivers.h:20,
                 from tcgbios.c:27:
tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
   92 |  } digestSizes[0];
      |    ^~~~~~~~~~~
---
 lib/libtpm/tcgbios_int.h | 2 +-
 lib/libtpm/tpm_drivers.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Thomas Huth July 1, 2021, 6:35 a.m. UTC | #1
On 30/06/2021 05.16, Alexey Kardashevskiy wrote:
> This fixes gcc warnings from -Waddress-of-packed-member and -Wzero-length-bounds.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> ---
> 
> tpm_drivers.c: In function ‘spapr_send_crq_and_wait’:
> tpm_drivers.c:153:2: warning: converting a packed ‘struct crq’ pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} pointer alignment 8) may result in an unaligned pointer value [-Waddress-of-packed-member]
>    153 |  rc = hv_send_crq(unit, (uint64_t *)crq);
>        |  ^~
> tpm_drivers.c:34:8: note: defined here
>     34 | struct crq {
>        |        ^~~
> tpm_drivers.c: In function ‘spapr_vtpm_senddata’:
> tpm_drivers.c:346:2: warning: converting a packed ‘struct crq’ pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} pointer (alignment 8) may result in an unaligned pointer value [-Waddress-of-packed-member]
>    346 |  rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
>        |  ^~
> tpm_drivers.c:34:8: note: defined here
>     34 | struct crq {
>        |        ^~~
>          [CC]    common-libs
>          [CC]    common-libs
> tcgbios.c: In function ‘tpm20_write_EfiSpecIdEventStruct’:
> tcgbios.c:708:24: warning: array subscript ‘numAlgs’ is outside the bounds of an interior zero-length array ‘struct TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>    708 |   event.hdr.digestSizes[numAlgs].algorithmId =
>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
> In file included from tpm_drivers.h:20,
>                   from tcgbios.c:27:
> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>     92 |  } digestSizes[0];
>        |    ^~~~~~~~~~~
> tcgbios.c:710:24: warning: array subscript ‘numAlgs’ is outside the bounds of an interior zero-length array ‘struct TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>    710 |   event.hdr.digestSizes[numAlgs].digestSize = cpu_to_log16(hsize);
>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
> In file included from tpm_drivers.h:20,
>                   from tcgbios.c:27:
> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>     92 |  } digestSizes[0];
>        |    ^~~~~~~~~~~
> ---
>   lib/libtpm/tcgbios_int.h | 2 +-
>   lib/libtpm/tpm_drivers.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/libtpm/tcgbios_int.h b/lib/libtpm/tcgbios_int.h
> index 22df31dd504f..cc38455850f2 100644
> --- a/lib/libtpm/tcgbios_int.h
> +++ b/lib/libtpm/tcgbios_int.h
> @@ -89,7 +89,7 @@ struct TCG_EfiSpecIdEventStruct {
>   	struct TCG_EfiSpecIdEventAlgorithmSize {
>   		uint16_t algorithmId;
>   		uint16_t digestSize;
> -	} digestSizes[0];
> +	} digestSizes[];
>   	/*
>   	uint8_t vendorInfoSize;
>   	uint8_t vendorInfo[0];
> diff --git a/lib/libtpm/tpm_drivers.c b/lib/libtpm/tpm_drivers.c
> index 85cb3098712d..4a4fde89ade8 100644
> --- a/lib/libtpm/tpm_drivers.c
> +++ b/lib/libtpm/tpm_drivers.c
> @@ -150,7 +150,7 @@ static bool spapr_send_crq_and_wait(unsigned long unit,
>   
>   	vtpm_drv_state_set(state1, VTPM_DRV_ERROR_NO_FAILURE);
>   
> -	rc = hv_send_crq(unit, (uint64_t *)crq);
> +	rc = hv_send_crq(unit, (uint64_t *)&crq->valid);
>   	if (rc != H_SUCCESS) {
>   		vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,
>   				   VTPM_DRV_ERROR_TPM_CRQ_ERROR);
> @@ -343,7 +343,7 @@ static bool spapr_vtpm_senddata(const uint8_t *const data, uint32_t len)
>   	vtpm_drv_state_set(VTPM_DRV_STATE_SEND_TPM_CMD,
>   			   VTPM_DRV_ERROR_NO_FAILURE);
>   
> -	rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
> +	rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq.valid);
>   
>   	if (rc == H_SUCCESS)
>   		vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_TPM_RSP,
> 

All members of struct crq seem to be naturally aligned, so you could also 
simply remove the "packed" attribute there.
Anyway, taking the detour via the uint8_t valid field seems ok to me, too, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>
Stefan Berger July 7, 2021, 3:49 p.m. UTC | #2
On 7/1/21 2:35 AM, Thomas Huth wrote:
> On 30/06/2021 05.16, Alexey Kardashevskiy wrote:
>> This fixes gcc warnings from -Waddress-of-packed-member and 
>> -Wzero-length-bounds.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> ---
>>
>> tpm_drivers.c: In function ‘spapr_send_crq_and_wait’:
>> tpm_drivers.c:153:2: warning: converting a packed ‘struct crq’ 
>> pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} 
>> pointer alignment 8) may result in an unaligned pointer value 
>> [-Waddress-of-packed-member]
>>    153 |  rc = hv_send_crq(unit, (uint64_t *)crq);
>>        |  ^~
>> tpm_drivers.c:34:8: note: defined here
>>     34 | struct crq {
>>        |        ^~~
>> tpm_drivers.c: In function ‘spapr_vtpm_senddata’:
>> tpm_drivers.c:346:2: warning: converting a packed ‘struct crq’ 
>> pointer (alignment 1) to a ‘uint64_t’ {aka ‘long long unsigned int’} 
>> pointer (alignment 8) may result in an unaligned pointer value 
>> [-Waddress-of-packed-member]
>>    346 |  rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
>>        |  ^~
>> tpm_drivers.c:34:8: note: defined here
>>     34 | struct crq {
>>        |        ^~~
>>          [CC]    common-libs
>>          [CC]    common-libs
>> tcgbios.c: In function ‘tpm20_write_EfiSpecIdEventStruct’:
>> tcgbios.c:708:24: warning: array subscript ‘numAlgs’ is outside the 
>> bounds of an interior zero-length array ‘struct 
>> TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>>    708 |   event.hdr.digestSizes[numAlgs].algorithmId =
>>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
>> In file included from tpm_drivers.h:20,
>>                   from tcgbios.c:27:
>> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>>     92 |  } digestSizes[0];
>>        |    ^~~~~~~~~~~
>> tcgbios.c:710:24: warning: array subscript ‘numAlgs’ is outside the 
>> bounds of an interior zero-length array ‘struct 
>> TCG_EfiSpecIdEventAlgorithmSize[0]’ [-Wzero-length-bounds]
>>    710 |   event.hdr.digestSizes[numAlgs].digestSize = 
>> cpu_to_log16(hsize);
>>        |   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
>> In file included from tpm_drivers.h:20,
>>                   from tcgbios.c:27:
>> tcgbios_int.h:92:4: note: while referencing ‘digestSizes’
>>     92 |  } digestSizes[0];
>>        |    ^~~~~~~~~~~
>> ---
>>   lib/libtpm/tcgbios_int.h | 2 +-
>>   lib/libtpm/tpm_drivers.c | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/libtpm/tcgbios_int.h b/lib/libtpm/tcgbios_int.h
>> index 22df31dd504f..cc38455850f2 100644
>> --- a/lib/libtpm/tcgbios_int.h
>> +++ b/lib/libtpm/tcgbios_int.h
>> @@ -89,7 +89,7 @@ struct TCG_EfiSpecIdEventStruct {
>>       struct TCG_EfiSpecIdEventAlgorithmSize {
>>           uint16_t algorithmId;
>>           uint16_t digestSize;
>> -    } digestSizes[0];
>> +    } digestSizes[];
>>       /*
>>       uint8_t vendorInfoSize;
>>       uint8_t vendorInfo[0];
>> diff --git a/lib/libtpm/tpm_drivers.c b/lib/libtpm/tpm_drivers.c
>> index 85cb3098712d..4a4fde89ade8 100644
>> --- a/lib/libtpm/tpm_drivers.c
>> +++ b/lib/libtpm/tpm_drivers.c
>> @@ -150,7 +150,7 @@ static bool spapr_send_crq_and_wait(unsigned long 
>> unit,
>>         vtpm_drv_state_set(state1, VTPM_DRV_ERROR_NO_FAILURE);
>>   -    rc = hv_send_crq(unit, (uint64_t *)crq);
>> +    rc = hv_send_crq(unit, (uint64_t *)&crq->valid);
>>       if (rc != H_SUCCESS) {
>>           vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,
>>                      VTPM_DRV_ERROR_TPM_CRQ_ERROR);
>> @@ -343,7 +343,7 @@ static bool spapr_vtpm_senddata(const uint8_t 
>> *const data, uint32_t len)
>>       vtpm_drv_state_set(VTPM_DRV_STATE_SEND_TPM_CMD,
>>                  VTPM_DRV_ERROR_NO_FAILURE);
>>   -    rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
>> +    rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq.valid);
>>         if (rc == H_SUCCESS)
>>           vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_TPM_RSP,
>>
>
> All members of struct crq seem to be naturally aligned, so you could 
> also simply remove the "packed" attribute there.
> Anyway, taking the detour via the uint8_t valid field seems ok to me, 
> too, so:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>


Tested-by: Stefan Berger <stefanb@linux.ibm.com>
diff mbox series

Patch

diff --git a/lib/libtpm/tcgbios_int.h b/lib/libtpm/tcgbios_int.h
index 22df31dd504f..cc38455850f2 100644
--- a/lib/libtpm/tcgbios_int.h
+++ b/lib/libtpm/tcgbios_int.h
@@ -89,7 +89,7 @@  struct TCG_EfiSpecIdEventStruct {
 	struct TCG_EfiSpecIdEventAlgorithmSize {
 		uint16_t algorithmId;
 		uint16_t digestSize;
-	} digestSizes[0];
+	} digestSizes[];
 	/*
 	uint8_t vendorInfoSize;
 	uint8_t vendorInfo[0];
diff --git a/lib/libtpm/tpm_drivers.c b/lib/libtpm/tpm_drivers.c
index 85cb3098712d..4a4fde89ade8 100644
--- a/lib/libtpm/tpm_drivers.c
+++ b/lib/libtpm/tpm_drivers.c
@@ -150,7 +150,7 @@  static bool spapr_send_crq_and_wait(unsigned long unit,
 
 	vtpm_drv_state_set(state1, VTPM_DRV_ERROR_NO_FAILURE);
 
-	rc = hv_send_crq(unit, (uint64_t *)crq);
+	rc = hv_send_crq(unit, (uint64_t *)&crq->valid);
 	if (rc != H_SUCCESS) {
 		vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,
 				   VTPM_DRV_ERROR_TPM_CRQ_ERROR);
@@ -343,7 +343,7 @@  static bool spapr_vtpm_senddata(const uint8_t *const data, uint32_t len)
 	vtpm_drv_state_set(VTPM_DRV_STATE_SEND_TPM_CMD,
 			   VTPM_DRV_ERROR_NO_FAILURE);
 
-	rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq);
+	rc = hv_send_crq(spapr_vtpm.unit, (uint64_t *)&crq.valid);
 
 	if (rc == H_SUCCESS)
 		vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_TPM_RSP,