Re: [PATCH 02/11] android/hidhost: Fix miscalculation of get report event struct length

[Szymon Janc <szymon.janc@tieto.com>]

Hi Ravi,

On Friday 17 of January 2014 01:25:42 Ravi kumar Veeramally wrote:
> ---
>  android/hal-hidhost.c | 3 ++-
>  android/hidhost.c     | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> index fd3ad2d..5445d08 100644
> --- a/android/hal-hidhost.c
> +++ b/android/hal-hidhost.c
> @@ -73,7 +73,8 @@ static void handle_get_report(void *buf, uint16_t len)
>  {
>  	struct hal_ev_hidhost_get_report *ev = buf;
>  
> -	if (len != sizeof(*ev) + ev->len) {
> +	if (len != sizeof(*ev) + sizeof(struct hal_ev_hidhost_get_report)
> +								+ ev->len) {

I don't understand this change. We have header and data len. There should be
no need for any extra calculations.


>  		error("invalid get report event, aborting");
>  		exit(EXIT_FAILURE);
>  	}
> diff --git a/android/hidhost.c b/android/hidhost.c
> index c004063..8a2668c 100644
> --- a/android/hidhost.c
> +++ b/android/hidhost.c
> @@ -371,13 +371,14 @@ static void bt_hid_notify_get_report(struct hid_device *dev, uint8_t *buf,
>  	ba2str(&dev->dst, address);
>  	DBG("device %s", address);
>  
> -	ev_len = sizeof(*ev) + sizeof(struct hal_ev_hidhost_get_report) + 1;
> +	ev_len = sizeof(*ev) + sizeof(struct hal_ev_hidhost_get_report);

I don't understand why there is double sizeof() in first place.(*ev is same type)

>  
>  	if (!((buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_INPUT)) ||
>  			(buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_OUTPUT)) ||
>  			(buf[0]	== (HID_MSG_DATA | HID_DATA_TYPE_FEATURE)))) {
>  		ev = g_malloc0(ev_len);
>  		ev->status = buf[0];
> +		ev->len = 0;
>  		bdaddr2android(&dev->dst, ev->bdaddr);
>  		goto send;
>  	}

I have a feeling that there is something wrong with ev_len calculations in
bt_hid_notify_get_report(). I'd rather prefer to have this function properly
fixed (and possibly refactored on how it handles ev data allocation).
 

-- 
Best regards, 
Szymon Janc