[Qemu-devel] [PATCH v2] bt: check struct sizes

[Qemu-devel] [PATCH v2] bt: check struct sizes

[Paolo Bonzini]

See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505.  For historical
reasons these do not use sizeof, and Coverity caught a mistake in
EVT_ENCRYPT_CHANGE_SIZE.

Note other sizes that seem wrong or inconsistent with the kernel header.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/bt.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/hw/bt.h b/include/hw/bt.h
index cb2a7e6..bc32807 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -507,6 +507,7 @@ typedef struct {
     uint8_t	status;
     bdaddr_t	bdaddr;
 } QEMU_PACKED create_conn_cancel_cp;
+/* FIXME: 6 or 7? Kernel doesn't have status??  */
 #define CREATE_CONN_CANCEL_CP_SIZE 6
 
 typedef struct {
@@ -1266,12 +1267,13 @@ typedef struct {
     uint8_t	status;
     uint16_t	handle;
 } QEMU_PACKED reset_failed_contact_counter_rp;
-#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 4
+#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 3
 
 #define OCF_READ_LINK_QUALITY		0x0003
 typedef struct {
     uint16_t	handle;
 } QEMU_PACKED read_link_quality_cp;
+/* FIXME: 2 or 4?? */
 #define READ_LINK_QUALITY_CP_SIZE 4
 
 typedef struct {
@@ -1332,6 +1334,7 @@ typedef struct {
     uint8_t	dev_class[3];
     uint16_t	clock_offset;
 } QEMU_PACKED inquiry_info;
+/* FIXME: 15 or 14??  Kernel doesn't have num_responses.  */
 #define INQUIRY_INFO_SIZE 14
 
 #define EVT_CONN_COMPLETE		0x03
@@ -1381,7 +1384,7 @@ typedef struct {
     uint16_t	handle;
     uint8_t	encrypt;
 } QEMU_PACKED evt_encrypt_change;
-#define EVT_ENCRYPT_CHANGE_SIZE 5
+#define EVT_ENCRYPT_CHANGE_SIZE 4
 
 #define EVT_CHANGE_CONN_LINK_KEY_COMPLETE	0x09
 typedef struct {
@@ -1570,6 +1573,7 @@ typedef struct {
     uint16_t	clock_offset;
     int8_t	rssi;
 } QEMU_PACKED inquiry_info_with_rssi;
+/* FIXME: 15 or 14??  Kernel doesn't have num_responses.  */
 #define INQUIRY_INFO_WITH_RSSI_SIZE 15
 typedef struct {
     uint8_t	num_responses;
@@ -1581,6 +1585,7 @@ typedef struct {
     uint16_t	clock_offset;
     int8_t	rssi;
 } QEMU_PACKED inquiry_info_with_rssi_and_pscan_mode;
+/* FIXME: 16 or 15??  Kernel doesn't have num_responses.  */
 #define INQUIRY_INFO_WITH_RSSI_AND_PSCAN_MODE_SIZE 16
 
 #define EVT_READ_REMOTE_EXT_FEATURES_COMPLETE	0x23
@@ -1639,6 +1644,7 @@ typedef struct {
     int8_t	rssi;
     uint8_t	data[240];
 } QEMU_PACKED extended_inquiry_info;
+/* FIXME: 254 or 255??  Should there be num_responses here too?  */
 #define EXTENDED_INQUIRY_INFO_SIZE 254
 
 #define EVT_TESTING			0xFE
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2] bt: check struct sizes

[Paolo Bonzini]

On 27/11/2015 18:57, Paolo Bonzini wrote:
> See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505.  For historical
> reasons these do not use sizeof, and Coverity caught a mistake in
> EVT_ENCRYPT_CHANGE_SIZE.
> 
> Note other sizes that seem wrong or inconsistent with the kernel header.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hmm, can do better...

Paolo

> ---
>  include/hw/bt.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/bt.h b/include/hw/bt.h
> index cb2a7e6..bc32807 100644
> --- a/include/hw/bt.h
> +++ b/include/hw/bt.h
> @@ -507,6 +507,7 @@ typedef struct {
>      uint8_t	status;
>      bdaddr_t	bdaddr;
>  } QEMU_PACKED create_conn_cancel_cp;
> +/* FIXME: 6 or 7? Kernel doesn't have status??  */
>  #define CREATE_CONN_CANCEL_CP_SIZE 6
>  
>  typedef struct {
> @@ -1266,12 +1267,13 @@ typedef struct {
>      uint8_t	status;
>      uint16_t	handle;
>  } QEMU_PACKED reset_failed_contact_counter_rp;
> -#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 4
> +#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 3
>  
>  #define OCF_READ_LINK_QUALITY		0x0003
>  typedef struct {
>      uint16_t	handle;
>  } QEMU_PACKED read_link_quality_cp;
> +/* FIXME: 2 or 4?? */
>  #define READ_LINK_QUALITY_CP_SIZE 4
>  
>  typedef struct {
> @@ -1332,6 +1334,7 @@ typedef struct {
>      uint8_t	dev_class[3];
>      uint16_t	clock_offset;
>  } QEMU_PACKED inquiry_info;
> +/* FIXME: 15 or 14??  Kernel doesn't have num_responses.  */
>  #define INQUIRY_INFO_SIZE 14
>  
>  #define EVT_CONN_COMPLETE		0x03
> @@ -1381,7 +1384,7 @@ typedef struct {
>      uint16_t	handle;
>      uint8_t	encrypt;
>  } QEMU_PACKED evt_encrypt_change;
> -#define EVT_ENCRYPT_CHANGE_SIZE 5
> +#define EVT_ENCRYPT_CHANGE_SIZE 4
>  
>  #define EVT_CHANGE_CONN_LINK_KEY_COMPLETE	0x09
>  typedef struct {
> @@ -1570,6 +1573,7 @@ typedef struct {
>      uint16_t	clock_offset;
>      int8_t	rssi;
>  } QEMU_PACKED inquiry_info_with_rssi;
> +/* FIXME: 15 or 14??  Kernel doesn't have num_responses.  */
>  #define INQUIRY_INFO_WITH_RSSI_SIZE 15
>  typedef struct {
>      uint8_t	num_responses;
> @@ -1581,6 +1585,7 @@ typedef struct {
>      uint16_t	clock_offset;
>      int8_t	rssi;
>  } QEMU_PACKED inquiry_info_with_rssi_and_pscan_mode;
> +/* FIXME: 16 or 15??  Kernel doesn't have num_responses.  */
>  #define INQUIRY_INFO_WITH_RSSI_AND_PSCAN_MODE_SIZE 16
>  
>  #define EVT_READ_REMOTE_EXT_FEATURES_COMPLETE	0x23
> @@ -1639,6 +1644,7 @@ typedef struct {
>      int8_t	rssi;
>      uint8_t	data[240];
>  } QEMU_PACKED extended_inquiry_info;
> +/* FIXME: 254 or 255??  Should there be num_responses here too?  */
>  #define EXTENDED_INQUIRY_INFO_SIZE 254
>  
>  #define EVT_TESTING			0xFE
> 

Re: [Qemu-devel] [PATCH v2] bt: check struct sizes

[Michael Tokarev]

28.11.2015 18:13, Paolo Bonzini wrote:
> On 27/11/2015 18:57, Paolo Bonzini wrote:
>> See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505.  For historical
>> reasons these do not use sizeof, and Coverity caught a mistake in
>> EVT_ENCRYPT_CHANGE_SIZE.
>>
>> Note other sizes that seem wrong or inconsistent with the kernel header.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Hmm, can do better...

Can do, or will do, or?
Note we've lost last S-o-b (by Markus) as well ;)

/mjt