[PATCH 1/2] Bluetooth: Don't increment twice in eir_has_data_type()

[PATCH 1/2] Bluetooth: Don't increment twice in eir_has_data_type()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The parsed variable is already incremented inside the for-loop so there
no need to increment it again (not to mention that the code was
incrementing it the wrong amount).

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8dc07fa..83cd301 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -911,11 +911,10 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
 
 static inline bool eir_has_data_type(u8 *data, size_t data_len, u8 type)
 {
-	u8 field_len;
-	size_t parsed;
+	size_t parsed = 0;
 
-	for (parsed = 0; parsed < data_len - 1; parsed += field_len) {
-		field_len = data[0];
+	while (parsed < data_len - 1) {
+		u8 field_len = data[0];
 
 		if (field_len == 0)
 			break;
-- 
1.7.9.1

[PATCH 2/2] Bluetooth: Check for minimum data length in eir_has_data_type()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

If passed 0 as data_length the (parsed < data_length - 1) test will be
true and cause a buffer overflow. In practice we need at least two bytes
for the element length and type so add a test for it to the very
beginning of the function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 83cd301..fa2c778 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -913,6 +913,9 @@ static inline bool eir_has_data_type(u8 *data, size_t data_len, u8 type)
 {
 	size_t parsed = 0;
 
+	if (data_len < 2)
+		return false;
+
 	while (parsed < data_len - 1) {
 		u8 field_len = data[0];
 
-- 
1.7.9.1

Re: [PATCH 1/2] Bluetooth: Don't increment twice in eir_has_data_type()

[Marcel Holtmann]

Hi Johan,

> The parsed variable is already incremented inside the for-loop so there
> no need to increment it again (not to mention that the code was
> incrementing it the wrong amount).
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 2/2] Bluetooth: Check for minimum data length in eir_has_data_type()

[Marcel Holtmann]

Hi Johan,

> If passed 0 as data_length the (parsed < data_length - 1) test will be
> true and cause a buffer overflow. In practice we need at least two bytes
> for the element length and type so add a test for it to the very
> beginning of the function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 2/2] Bluetooth: Check for minimum data length in eir_has_data_type()

[Gustavo Padovan]

Hi Johan,

* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2012-03-26 14:21:42 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> If passed 0 as data_length the (parsed < data_length - 1) test will be
> true and cause a buffer overflow. In practice we need at least two bytes
> for the element length and type so add a test for it to the very
> beginning of the function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

I applied both patches to bluetooth-next

	Gustavo