All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks
@ 2013-01-18 13:25 Johan Hedberg
  2013-01-18 13:25 ` [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters Johan Hedberg
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This patch set is intended to be applied after Szymon's "Bluetooth: Fix
pair device command reply if adapter is powered off" patch.

This set includes fixes to improve the validity checks on parameters we
receive from user space in mgmt commands. Each of these fixes has had
its own test case added to the user space mgmt-tester and has been
verified to work.

Johan


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 16:55   ` Marcel Holtmann
  2013-01-18 13:25 ` [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs Johan Hedberg
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

The debug_keys parameter is only allowed to have the values 0x00 and
0x01. Any other value should result in a proper command status with
MGMT_STATUS_INVALID_PARAMS.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 36b2310..d9b042e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1519,6 +1519,10 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
 				  MGMT_STATUS_INVALID_PARAMS);
 	}
 
+	if (cp->debug_keys != 0x00 && cp->debug_keys != 0x01)
+		return cmd_status(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS,
+				  MGMT_STATUS_INVALID_PARAMS);
+
 	BT_DBG("%s debug_keys %u key_count %u", hdev->name, cp->debug_keys,
 	       key_count);
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
  2013-01-18 13:25 ` [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 16:55   ` Marcel Holtmann
  2013-01-18 13:25 ` [PATCH 03/10] Bluetooth: Fix checking for proper key->master value in " Johan Hedberg
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

Failures of mgmt commands should be indicated with valid mgmt status
codes, and EINVAL is not one of them. Instead MGMT_STATUS_INVALID_PARAMS
should be returned.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d9b042e..a050eee 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2716,7 +2716,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 		BT_ERR("load_keys: expected %u bytes, got %u bytes",
 		       len, expected_len);
 		return cmd_status(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS,
-				  EINVAL);
+				  MGMT_STATUS_INVALID_PARAMS);
 	}
 
 	BT_DBG("%s key_count %u", hdev->name, key_count);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 03/10] Bluetooth: Fix checking for proper key->master value in Load LTKs
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
  2013-01-18 13:25 ` [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters Johan Hedberg
  2013-01-18 13:25 ` [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 13:25 ` [PATCH 04/10] Bluetooth: Refactor valid LTK data testing into its own function Johan Hedberg
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

The allowed values for the key->master parameter in the Load LTKs
command are 0x00 and 0x01. If there is a key in the list with some other
value the command should fail with a proper invalid params response.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a050eee..5388151 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2729,6 +2729,14 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 		struct mgmt_ltk_info *key = &cp->keys[i];
 		u8 type;
 
+		if (key->master != 0x00 && key->master != 0x01) {
+			hci_smp_ltks_clear(hdev);
+			err = cmd_status(sk, hdev->id,
+					 MGMT_OP_LOAD_LONG_TERM_KEYS,
+					 MGMT_STATUS_INVALID_PARAMS);
+			goto unlock;
+		}
+
 		if (key->master)
 			type = HCI_SMP_LTK;
 		else
@@ -2743,6 +2751,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 	err = cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS, 0,
 			   NULL, 0);
 
+unlock:
 	hci_dev_unlock(hdev);
 
 	return err;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 04/10] Bluetooth: Refactor valid LTK data testing into its own function
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (2 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 03/10] Bluetooth: Fix checking for proper key->master value in " Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 13:25 ` [PATCH 05/10] Bluetooth: Check for valid key->authenticated value for LTKs Johan Hedberg
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

This patch refactors valid LTK data testing into its own function. This
will help keep the code readable since there are several tests still
missing that need to be done on the LTK data.
---
 net/bluetooth/mgmt.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5388151..3634907 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2701,6 +2701,13 @@ done:
 	return err;
 }
 
+static bool ltk_is_valid(struct mgmt_ltk_info *key)
+{
+	if (key->master != 0x00 && key->master != 0x01)
+		return false;
+	return true;
+}
+
 static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 			       void *cp_data, u16 len)
 {
@@ -2729,7 +2736,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 		struct mgmt_ltk_info *key = &cp->keys[i];
 		u8 type;
 
-		if (key->master != 0x00 && key->master != 0x01) {
+		if (!ltk_is_valid(key)) {
 			hci_smp_ltks_clear(hdev);
 			err = cmd_status(sk, hdev->id,
 					 MGMT_OP_LOAD_LONG_TERM_KEYS,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 05/10] Bluetooth: Check for valid key->authenticated value for LTKs
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (3 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 04/10] Bluetooth: Refactor valid LTK data testing into its own function Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 13:25 ` [PATCH 06/10] Bluetooth: Add helper functions for testing bdaddr types Johan Hedberg
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

This patch adds necessary checks for the two allowed values of the
authenticated parameter of each Long Term Key, i.e. 0x00 and 0x01. If
any other value is encountered the valid response is to return invalid
params to user space.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3634907..76301a3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2703,6 +2703,8 @@ done:
 
 static bool ltk_is_valid(struct mgmt_ltk_info *key)
 {
+	if (key->authenticated != 0x00 && key->authenticated != 0x01)
+		return false;
 	if (key->master != 0x00 && key->master != 0x01)
 		return false;
 	return true;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 06/10] Bluetooth: Add helper functions for testing bdaddr types
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (4 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 05/10] Bluetooth: Check for valid key->authenticated value for LTKs Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 13:25 ` [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands Johan Hedberg
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

This patch adds two helper functions to test for valid bdaddr type
values. These will be particularely useful in the mgmt code to check
that user space has passed valid values to the kernel.

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

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 2554b3f..9531bee 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -166,6 +166,29 @@ typedef struct {
 #define BDADDR_LE_PUBLIC	0x01
 #define BDADDR_LE_RANDOM	0x02
 
+static inline bool bdaddr_type_is_valid(__u8 type)
+{
+	switch (type) {
+	case BDADDR_BREDR:
+	case BDADDR_LE_PUBLIC:
+	case BDADDR_LE_RANDOM:
+		return true;
+	}
+
+	return false;
+}
+
+static inline bool bdaddr_type_is_le(__u8 type)
+{
+	switch (type) {
+	case BDADDR_LE_PUBLIC:
+	case BDADDR_LE_RANDOM:
+		return true;
+	}
+
+	return false;
+}
+
 #define BDADDR_ANY   (&(bdaddr_t) {{0, 0, 0, 0, 0, 0} })
 #define BDADDR_LOCAL (&(bdaddr_t) {{0, 0, 0, 0xff, 0xff, 0xff} })
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (5 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 06/10] Bluetooth: Add helper functions for testing bdaddr types Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 16:53   ` Marcel Holtmann
  2013-01-18 13:25 ` [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device Johan Hedberg
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

This patch adds checks for valid address type values passed to mgmt
commands. If an invalid address type is encountered the code will return
a proper invalid params response.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 76301a3..3de4bc2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1506,7 +1506,7 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
 {
 	struct mgmt_cp_load_link_keys *cp = data;
 	u16 key_count, expected_len;
-	int i;
+	int i, err;
 
 	key_count = __le16_to_cpu(cp->key_count);
 
@@ -1540,15 +1540,24 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
 	for (i = 0; i < key_count; i++) {
 		struct mgmt_link_key_info *key = &cp->keys[i];
 
+		if (key->addr.type != BDADDR_BREDR) {
+			clear_bit(HCI_DEBUG_KEYS, &hdev->dev_flags);
+			hci_link_keys_clear(hdev);
+			err = cmd_status(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS,
+					 MGMT_STATUS_INVALID_PARAMS);
+			goto unlock;
+		}
+
 		hci_add_link_key(hdev, NULL, 0, &key->addr.bdaddr, key->val,
 				 key->type, key->pin_len);
 	}
 
-	cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS, 0, NULL, 0);
+	err = cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS, 0, NULL, 0);
 
+unlock:
 	hci_dev_unlock(hdev);
 
-	return 0;
+	return err;
 }
 
 static int device_unpaired(struct hci_dev *hdev, bdaddr_t *bdaddr,
@@ -1573,12 +1582,17 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct hci_conn *conn;
 	int err;
 
-	hci_dev_lock(hdev);
-
 	memset(&rp, 0, sizeof(rp));
 	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
 	rp.addr.type = cp->addr.type;
 
+	if (!bdaddr_type_is_valid(cp->addr.type))
+		return cmd_complete(sk, hdev->id, MGMT_OP_UNPAIR_DEVICE,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &rp, sizeof(rp));
+
+	hci_dev_lock(hdev);
+
 	if (!hdev_is_powered(hdev)) {
 		err = cmd_complete(sk, hdev->id, MGMT_OP_UNPAIR_DEVICE,
 				   MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
@@ -1643,6 +1657,10 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("");
 
+	if (!bdaddr_type_is_valid(cp->addr.type))
+		return cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
+				  MGMT_STATUS_INVALID_PARAMS);
+
 	hci_dev_lock(hdev);
 
 	if (!test_bit(HCI_UP, &hdev->flags)) {
@@ -1947,6 +1965,11 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
 	rp.addr.type = cp->addr.type;
 
+	if (!bdaddr_type_is_valid(cp->addr.type))
+		return cmd_complete(sk, hdev->id, MGMT_OP_PAIR_DEVICE,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &rp, sizeof(rp));
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
@@ -2564,6 +2587,10 @@ static int block_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("%s", hdev->name);
 
+	if (!bdaddr_type_is_valid(cp->addr.type))
+		return cmd_status(sk, hdev->id, MGMT_OP_BLOCK_DEVICE,
+				  MGMT_STATUS_INVALID_PARAMS);
+
 	hci_dev_lock(hdev);
 
 	err = hci_blacklist_add(hdev, &cp->addr.bdaddr, cp->addr.type);
@@ -2589,6 +2616,10 @@ static int unblock_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("%s", hdev->name);
 
+	if (!bdaddr_type_is_valid(cp->addr.type))
+		return cmd_status(sk, hdev->id, MGMT_OP_UNBLOCK_DEVICE,
+				  MGMT_STATUS_INVALID_PARAMS);
+
 	hci_dev_lock(hdev);
 
 	err = hci_blacklist_del(hdev, &cp->addr.bdaddr, cp->addr.type);
@@ -2707,6 +2738,8 @@ static bool ltk_is_valid(struct mgmt_ltk_info *key)
 		return false;
 	if (key->master != 0x00 && key->master != 0x01)
 		return false;
+	if (!bdaddr_type_is_le(key->addr.type))
+		return false;
 	return true;
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (6 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 16:54   ` Marcel Holtmann
  2013-01-18 13:25 ` [PATCH 09/10] Bluetooth: Fix returning proper cmd_complete for mgmt_disconnect Johan Hedberg
  2013-01-18 13:25 ` [PATCH 10/10] Bluetooth: Fix returning proper cmd_complete for mgmt_block/unblock Johan Hedberg
  9 siblings, 1 reply; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

The valid values for the Disconnect parameter in the Unpair Device
command are 0x00 and 0x01. If any other value is encountered the command
should fail with the appropriate invalid params response.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3de4bc2..8ec3c4f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1591,6 +1591,11 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 				    MGMT_STATUS_INVALID_PARAMS,
 				    &rp, sizeof(rp));
 
+	if (cp->disconnect != 0x00 && cp->disconnect != 0x01)
+		return cmd_complete(sk, hdev->id, MGMT_OP_UNPAIR_DEVICE,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &rp, sizeof(rp));
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 09/10] Bluetooth: Fix returning proper cmd_complete for mgmt_disconnect
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (7 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  2013-01-18 13:25 ` [PATCH 10/10] Bluetooth: Fix returning proper cmd_complete for mgmt_block/unblock Johan Hedberg
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

The Disconnect Management command should return Command Complete instead
of Command Status whenever possible so that user space can distinguish
exactly which command failed in the case of multiple commands. This
patch does the necessary changes in the disconnect command handler to
return the right event to user space.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 8ec3c4f..184673e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1655,6 +1655,7 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 		      u16 len)
 {
 	struct mgmt_cp_disconnect *cp = data;
+	struct mgmt_rp_disconnect rp;
 	struct hci_cp_disconnect dc;
 	struct pending_cmd *cmd;
 	struct hci_conn *conn;
@@ -1662,21 +1663,26 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("");
 
+	memset(&rp, 0, sizeof(rp));
+	bacpy(&rp.addr.bdaddr, &cp->addr.bdaddr);
+	rp.addr.type = cp->addr.type;
+
 	if (!bdaddr_type_is_valid(cp->addr.type))
-		return cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
-				  MGMT_STATUS_INVALID_PARAMS);
+		return cmd_complete(sk, hdev->id, MGMT_OP_DISCONNECT,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &rp, sizeof(rp));
 
 	hci_dev_lock(hdev);
 
 	if (!test_bit(HCI_UP, &hdev->flags)) {
-		err = cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
-				 MGMT_STATUS_NOT_POWERED);
+		err = cmd_complete(sk, hdev->id, MGMT_OP_DISCONNECT,
+				   MGMT_STATUS_NOT_POWERED, &rp, sizeof(rp));
 		goto failed;
 	}
 
 	if (mgmt_pending_find(MGMT_OP_DISCONNECT, hdev)) {
-		err = cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
-				 MGMT_STATUS_BUSY);
+		err = cmd_complete(sk, hdev->id, MGMT_OP_DISCONNECT,
+				   MGMT_STATUS_BUSY, &rp, sizeof(rp));
 		goto failed;
 	}
 
@@ -1687,8 +1693,8 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
 		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
 
 	if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {
-		err = cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
-				 MGMT_STATUS_NOT_CONNECTED);
+		err = cmd_complete(sk, hdev->id, MGMT_OP_DISCONNECT,
+				   MGMT_STATUS_NOT_CONNECTED, &rp, sizeof(rp));
 		goto failed;
 	}
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 10/10] Bluetooth: Fix returning proper cmd_complete for mgmt_block/unblock
  2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
                   ` (8 preceding siblings ...)
  2013-01-18 13:25 ` [PATCH 09/10] Bluetooth: Fix returning proper cmd_complete for mgmt_disconnect Johan Hedberg
@ 2013-01-18 13:25 ` Johan Hedberg
  9 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2013-01-18 13:25 UTC (permalink / raw)
  To: linux-bluetooth

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

The Block/Unblock Device Management commands should return Command
Complete instead of Command Status whenever possible so that user space
can distinguish exactly which command failed in the case of multiple
commands. This patch does the necessary changes in the command handler
to return the right event to user space.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 184673e..50daae8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2599,8 +2599,9 @@ static int block_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	BT_DBG("%s", hdev->name);
 
 	if (!bdaddr_type_is_valid(cp->addr.type))
-		return cmd_status(sk, hdev->id, MGMT_OP_BLOCK_DEVICE,
-				  MGMT_STATUS_INVALID_PARAMS);
+		return cmd_complete(sk, hdev->id, MGMT_OP_BLOCK_DEVICE,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &cp->addr, sizeof(cp->addr));
 
 	hci_dev_lock(hdev);
 
@@ -2628,8 +2629,9 @@ static int unblock_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	BT_DBG("%s", hdev->name);
 
 	if (!bdaddr_type_is_valid(cp->addr.type))
-		return cmd_status(sk, hdev->id, MGMT_OP_UNBLOCK_DEVICE,
-				  MGMT_STATUS_INVALID_PARAMS);
+		return cmd_complete(sk, hdev->id, MGMT_OP_UNBLOCK_DEVICE,
+				    MGMT_STATUS_INVALID_PARAMS,
+				    &cp->addr, sizeof(cp->addr));
 
 	hci_dev_lock(hdev);
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands
  2013-01-18 13:25 ` [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands Johan Hedberg
@ 2013-01-18 16:53   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2013-01-18 16:53 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> This patch adds checks for valid address type values passed to mgmt
> commands. If an invalid address type is encountered the code will return
> a proper invalid params response.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 76301a3..3de4bc2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1506,7 +1506,7 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
>  {
>  	struct mgmt_cp_load_link_keys *cp = data;
>  	u16 key_count, expected_len;
> -	int i;
> +	int i, err;
>  
>  	key_count = __le16_to_cpu(cp->key_count);
>  
> @@ -1540,15 +1540,24 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
>  	for (i = 0; i < key_count; i++) {
>  		struct mgmt_link_key_info *key = &cp->keys[i];
>  
> +		if (key->addr.type != BDADDR_BREDR) {
> +			clear_bit(HCI_DEBUG_KEYS, &hdev->dev_flags);
> +			hci_link_keys_clear(hdev);
> +			err = cmd_status(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS,
> +					 MGMT_STATUS_INVALID_PARAMS);
> +			goto unlock;
> +		}
> +
>  		hci_add_link_key(hdev, NULL, 0, &key->addr.bdaddr, key->val,
>  				 key->type, key->pin_len);
>  	}

I am not a huge fan of doing it this way (same applies to LTKs). You are
introducing a side effect here. An invalid command will clear the keys.

Please check the validity of the key parameters ahead of time and not in
the middle of applying them. Your rollback is broken since it just
clears all keys.

We could add /sys/kernel/debug/bluetooth/hci0/link_keys as a root-only
entry to allow checking this mgmt-tester.

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device
  2013-01-18 13:25 ` [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device Johan Hedberg
@ 2013-01-18 16:54   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2013-01-18 16:54 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> The valid values for the Disconnect parameter in the Unpair Device
> command are 0x00 and 0x01. If any other value is encountered the command
> should fail with the appropriate invalid params response.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    5 +++++
>  1 file changed, 5 insertions(+)

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

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters
  2013-01-18 13:25 ` [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters Johan Hedberg
@ 2013-01-18 16:55   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2013-01-18 16:55 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> The debug_keys parameter is only allowed to have the values 0x00 and
> 0x01. Any other value should result in a proper command status with
> MGMT_STATUS_INVALID_PARAMS.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    4 ++++
>  1 file changed, 4 insertions(+)

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

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs
  2013-01-18 13:25 ` [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs Johan Hedberg
@ 2013-01-18 16:55   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2013-01-18 16:55 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> Failures of mgmt commands should be indicated with valid mgmt status
> codes, and EINVAL is not one of them. Instead MGMT_STATUS_INVALID_PARAMS
> should be returned.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-01-18 16:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-18 13:25 [PATCH 00/10] Bluetooth: Fix more mgmt parameter checks Johan Hedberg
2013-01-18 13:25 ` [PATCH 01/10] Bluetooth: Fix checking for correct mgmt_load_link_keys parameters Johan Hedberg
2013-01-18 16:55   ` Marcel Holtmann
2013-01-18 13:25 ` [PATCH 02/10] Bluetooth: Fix returning proper mgmt status for Load LTKs Johan Hedberg
2013-01-18 16:55   ` Marcel Holtmann
2013-01-18 13:25 ` [PATCH 03/10] Bluetooth: Fix checking for proper key->master value in " Johan Hedberg
2013-01-18 13:25 ` [PATCH 04/10] Bluetooth: Refactor valid LTK data testing into its own function Johan Hedberg
2013-01-18 13:25 ` [PATCH 05/10] Bluetooth: Check for valid key->authenticated value for LTKs Johan Hedberg
2013-01-18 13:25 ` [PATCH 06/10] Bluetooth: Add helper functions for testing bdaddr types Johan Hedberg
2013-01-18 13:25 ` [PATCH 07/10] Bluetooth: Fix checking for valid address type values in mgmt commands Johan Hedberg
2013-01-18 16:53   ` Marcel Holtmann
2013-01-18 13:25 ` [PATCH 08/10] Bluetooth: Fix checking for valid disconnect parameters in unpair_device Johan Hedberg
2013-01-18 16:54   ` Marcel Holtmann
2013-01-18 13:25 ` [PATCH 09/10] Bluetooth: Fix returning proper cmd_complete for mgmt_disconnect Johan Hedberg
2013-01-18 13:25 ` [PATCH 10/10] Bluetooth: Fix returning proper cmd_complete for mgmt_block/unblock Johan Hedberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.