[PATCH] Bluetooth: Padding should be removed from EIR data

[PATCH] Bluetooth: Padding should be removed from EIR data

[Vishal Agarwal]

EIR data received from controller might contain padding zeros.
This padding should be removed before any further processing and
sending it over mgmt interface.

Signed-off-by: Vishal Agarwal <vishal.agarwal@stericsson.com>
---
 include/net/bluetooth/hci_core.h |   17 +++++++++++++++++
 net/bluetooth/hci_event.c        |    4 +++-
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ef6e654..eec721c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -935,6 +935,23 @@ static inline bool eir_has_data_type(u8 *data, size_t data_len, u8 type)
 	return false;
 }
 
+static inline size_t eir_get_padding_offset(u8 *eir, size_t eir_len)
+{
+	u8 field_len;
+	size_t parsed = 0;
+
+	while (parsed < eir_len) {
+		field_len = eir[0];
+
+		if (field_len == 0)
+			return parsed;
+
+		parsed += field_len + 1;
+		eir += field_len + 1;
+	}
+	return eir_len;
+}
+
 static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type, u8 *data,
 				  u8 data_len)
 {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index fb23c47..067464f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3000,6 +3000,7 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 	struct inquiry_data data;
 	struct extended_inquiry_info *info = (void *) (skb->data + 1);
 	int num_rsp = *((__u8 *) skb->data);
+	size_t eir_len;
 
 	BT_DBG("%s num_rsp %d", hdev->name, num_rsp);
 
@@ -3032,9 +3033,10 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 
 		name_known = hci_inquiry_cache_update(hdev, &data, name_known,
 						      &ssp);
+		eir_len = eir_get_padding_offset(info->data, sizeof(info->data));
 		mgmt_device_found(hdev, &info->bdaddr, ACL_LINK, 0x00,
 				  info->dev_class, info->rssi, !name_known,
-				  ssp, info->data, sizeof(info->data));
+				  ssp, info->data, eir_len);
 	}
 
 	hci_dev_unlock(hdev);
-- 
1.7.0.4

Re: [PATCH] Bluetooth: Padding should be removed from EIR data

[Johan Hedberg]

Hi Vishal,

On Wed, Apr 25, 2012, Vishal Agarwal wrote:
> EIR data received from controller might contain padding zeros.
> This padding should be removed before any further processing and
> sending it over mgmt interface.
> 
> Signed-off-by: Vishal Agarwal <vishal.agarwal@stericsson.com>
> ---
>  include/net/bluetooth/hci_core.h |   17 +++++++++++++++++
>  net/bluetooth/hci_event.c        |    4 +++-
>  2 files changed, 20 insertions(+), 1 deletions(-)

Could you please prefix the summary line as Fix... It makes it easier to
get this to 3.4 as this really must go there. Also explain in the commit
message that the mgmt_device_found expects to receive only the
significant part of the EIR data.

> +static inline size_t eir_get_padding_offset(u8 *eir, size_t eir_len)

I'm not completely sure about this name. Would eir_significant_len()
sound better? The core spec uses the terminology "significant part" and
non-significant part" to refer to this so I thought reusing that might
make it clearer what we're dealing with.

> +	u8 field_len;
> +	size_t parsed = 0;
> +
> +	while (parsed < eir_len) {
> +		field_len = eir[0];

You should define field_len here instead of the beginning of the
function since it's not used outside of the while-loop.

> +
> +		if (field_len == 0)
> +			return parsed;
> +
> +		parsed += field_len + 1;
> +		eir += field_len + 1;
> +	}
> +	return eir_len;

Add an empty line before the return statement please.

Johan

Re: [PATCH] Bluetooth: Padding should be removed from EIR data

[Johan Hedberg]

Hi Vishal,

On Thu, Apr 26, 2012, Johan Hedberg wrote:
> > +static inline size_t eir_get_padding_offset(u8 *eir, size_t eir_len)
> 
> I'm not completely sure about this name. Would eir_significant_len()
> sound better? The core spec uses the terminology "significant part" and
> non-significant part" to refer to this so I thought reusing that might
> make it clearer what we're dealing with.

I changed my mind: let's just call it eir_get_length(). And to make it
more clear call the input parameters data and data_len.

Johan