[v5,5/5] Bluetooth: Change MGMT security info CMD to be more generic
diff mbox series

Message ID 20201124100610.v5.5.I5068c01cae3cea674a96e103a0cf4d8c81425a4f@changeid
State New, archived
Headers show
Series
  • Bluetooth: Add new MGMT interface for advertising add
Related show

Commit Message

Daniel Winkler Nov. 24, 2020, 6:07 p.m. UTC
For advertising, we wish to know the LE tx power capabilities of the
controller in userspace, so this patch edits the Security Info MGMT
command to be more generic, such that other various controller
capabilities can be included in the EIR data. This change also includes
the LE min and max tx power into this newly-named command.

The change was tested by manually verifying that the MGMT command
returns the tx power range as expected in userspace.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Signed-off-by: Daniel Winkler <danielwinkler@google.com>
---

Changes in v5: None
Changes in v4:
- Combine LE tx range into a single EIR field for MGMT capabilities cmd

Changes in v3:
- Re-using security info MGMT command to carry controller capabilities

Changes in v2:
- Fixed sparse error in Capabilities MGMT command

 include/net/bluetooth/mgmt.h | 15 +++++++++-----
 net/bluetooth/mgmt.c         | 39 +++++++++++++++++++++++-------------
 2 files changed, 35 insertions(+), 19 deletions(-)

Comments

Marcel Holtmann Nov. 25, 2020, 2:35 p.m. UTC | #1
Hi Daniel,

> For advertising, we wish to know the LE tx power capabilities of the
> controller in userspace, so this patch edits the Security Info MGMT
> command to be more generic, such that other various controller
> capabilities can be included in the EIR data. This change also includes
> the LE min and max tx power into this newly-named command.
> 
> The change was tested by manually verifying that the MGMT command
> returns the tx power range as expected in userspace.
> 
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> Signed-off-by: Daniel Winkler <danielwinkler@google.com>
> ---
> 
> Changes in v5: None
> Changes in v4:
> - Combine LE tx range into a single EIR field for MGMT capabilities cmd
> 
> Changes in v3:
> - Re-using security info MGMT command to carry controller capabilities
> 
> Changes in v2:
> - Fixed sparse error in Capabilities MGMT command
> 
> include/net/bluetooth/mgmt.h | 15 +++++++++-----
> net/bluetooth/mgmt.c         | 39 +++++++++++++++++++++++-------------
> 2 files changed, 35 insertions(+), 19 deletions(-)
> 
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 2e18e4173e2fa5..f9a6638e20b3c6 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -686,11 +686,16 @@ struct mgmt_cp_set_blocked_keys {
> 
> #define MGMT_OP_SET_WIDEBAND_SPEECH	0x0047
> 
> -#define MGMT_OP_READ_SECURITY_INFO	0x0048
> -#define MGMT_READ_SECURITY_INFO_SIZE	0
> -struct mgmt_rp_read_security_info {
> -	__le16   sec_len;
> -	__u8     sec[];
> +#define MGMT_CAP_SEC_FLAGS		0x01
> +#define MGMT_CAP_MAX_ENC_KEY_SIZE	0x02
> +#define MGMT_CAP_SMP_MAX_ENC_KEY_SIZE	0x03
> +#define MGMT_CAP_LE_TX_PWR		0x04
> +
> +#define MGMT_OP_READ_CONTROLLER_CAP	0x0048
> +#define MGMT_READ_CONTROLLER_CAP_SIZE	0
> +struct mgmt_rp_read_controller_cap {
> +	__le16   cap_len;
> +	__u8     cap[0];
> } __packed;
> 
> #define MGMT_OP_READ_EXP_FEATURES_INFO	0x0049
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 668a62c8181eb1..d8adf78a437e0b 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -110,7 +110,7 @@ static const u16 mgmt_commands[] = {
> 	MGMT_OP_SET_APPEARANCE,
> 	MGMT_OP_SET_BLOCKED_KEYS,
> 	MGMT_OP_SET_WIDEBAND_SPEECH,
> -	MGMT_OP_READ_SECURITY_INFO,
> +	MGMT_OP_READ_CONTROLLER_CAP,
> 	MGMT_OP_READ_EXP_FEATURES_INFO,
> 	MGMT_OP_SET_EXP_FEATURE,
> 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> @@ -176,7 +176,7 @@ static const u16 mgmt_untrusted_commands[] = {
> 	MGMT_OP_READ_CONFIG_INFO,
> 	MGMT_OP_READ_EXT_INDEX_LIST,
> 	MGMT_OP_READ_EXT_INFO,
> -	MGMT_OP_READ_SECURITY_INFO,
> +	MGMT_OP_READ_CONTROLLER_CAP,
> 	MGMT_OP_READ_EXP_FEATURES_INFO,
> 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> 	MGMT_OP_READ_DEF_RUNTIME_CONFIG,
> @@ -3710,13 +3710,14 @@ static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
> 	return err;
> }
> 
> -static int read_security_info(struct sock *sk, struct hci_dev *hdev,
> -			      void *data, u16 data_len)
> +static int read_controller_cap(struct sock *sk, struct hci_dev *hdev,
> +			       void *data, u16 data_len)
> {
> -	char buf[16];
> -	struct mgmt_rp_read_security_info *rp = (void *)buf;
> -	u16 sec_len = 0;
> +	char buf[20];
> +	struct mgmt_rp_read_controller_cap *rp = (void *)buf;
> +	u16 cap_len = 0;
> 	u8 flags = 0;
> +	u8 tx_power_range[2];
> 
> 	bt_dev_dbg(hdev, "sock %p", sk);
> 
> @@ -3740,23 +3741,33 @@ static int read_security_info(struct sock *sk, struct hci_dev *hdev,
> 
> 	flags |= 0x08;		/* Encryption key size enforcement (LE) */
> 
> -	sec_len = eir_append_data(rp->sec, sec_len, 0x01, &flags, 1);
> +	cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_SEC_FLAGS,
> +				  &flags, 1);
> 
> 	/* When the Read Simple Pairing Options command is supported, then
> 	 * also max encryption key size information is provided.
> 	 */
> 	if (hdev->commands[41] & 0x08)
> -		sec_len = eir_append_le16(rp->sec, sec_len, 0x02,
> +		cap_len = eir_append_le16(rp->cap, cap_len,
> +					  MGMT_CAP_MAX_ENC_KEY_SIZE,
> 					  hdev->max_enc_key_size);
> 
> -	sec_len = eir_append_le16(rp->sec, sec_len, 0x03, SMP_MAX_ENC_KEY_SIZE);
> +	cap_len = eir_append_le16(rp->cap, cap_len,
> +				  MGMT_CAP_SMP_MAX_ENC_KEY_SIZE,
> +				  SMP_MAX_ENC_KEY_SIZE);
> +
> +	/* Append the min/max LE tx power parameters */
> +	memcpy(&tx_power_range[0], &hdev->min_le_tx_power, 1);
> +	memcpy(&tx_power_range[1], &hdev->max_le_tx_power, 1);
> +	cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_LE_TX_PWR,
> +				  tx_power_range, 2);

this is not enough. You need to also factor in if the command is supported or not. If the command is not supported we are not providing this field.

Regards

Marcel

Patch
diff mbox series

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 2e18e4173e2fa5..f9a6638e20b3c6 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -686,11 +686,16 @@  struct mgmt_cp_set_blocked_keys {
 
 #define MGMT_OP_SET_WIDEBAND_SPEECH	0x0047
 
-#define MGMT_OP_READ_SECURITY_INFO	0x0048
-#define MGMT_READ_SECURITY_INFO_SIZE	0
-struct mgmt_rp_read_security_info {
-	__le16   sec_len;
-	__u8     sec[];
+#define MGMT_CAP_SEC_FLAGS		0x01
+#define MGMT_CAP_MAX_ENC_KEY_SIZE	0x02
+#define MGMT_CAP_SMP_MAX_ENC_KEY_SIZE	0x03
+#define MGMT_CAP_LE_TX_PWR		0x04
+
+#define MGMT_OP_READ_CONTROLLER_CAP	0x0048
+#define MGMT_READ_CONTROLLER_CAP_SIZE	0
+struct mgmt_rp_read_controller_cap {
+	__le16   cap_len;
+	__u8     cap[0];
 } __packed;
 
 #define MGMT_OP_READ_EXP_FEATURES_INFO	0x0049
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 668a62c8181eb1..d8adf78a437e0b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -110,7 +110,7 @@  static const u16 mgmt_commands[] = {
 	MGMT_OP_SET_APPEARANCE,
 	MGMT_OP_SET_BLOCKED_KEYS,
 	MGMT_OP_SET_WIDEBAND_SPEECH,
-	MGMT_OP_READ_SECURITY_INFO,
+	MGMT_OP_READ_CONTROLLER_CAP,
 	MGMT_OP_READ_EXP_FEATURES_INFO,
 	MGMT_OP_SET_EXP_FEATURE,
 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
@@ -176,7 +176,7 @@  static const u16 mgmt_untrusted_commands[] = {
 	MGMT_OP_READ_CONFIG_INFO,
 	MGMT_OP_READ_EXT_INDEX_LIST,
 	MGMT_OP_READ_EXT_INFO,
-	MGMT_OP_READ_SECURITY_INFO,
+	MGMT_OP_READ_CONTROLLER_CAP,
 	MGMT_OP_READ_EXP_FEATURES_INFO,
 	MGMT_OP_READ_DEF_SYSTEM_CONFIG,
 	MGMT_OP_READ_DEF_RUNTIME_CONFIG,
@@ -3710,13 +3710,14 @@  static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
-static int read_security_info(struct sock *sk, struct hci_dev *hdev,
-			      void *data, u16 data_len)
+static int read_controller_cap(struct sock *sk, struct hci_dev *hdev,
+			       void *data, u16 data_len)
 {
-	char buf[16];
-	struct mgmt_rp_read_security_info *rp = (void *)buf;
-	u16 sec_len = 0;
+	char buf[20];
+	struct mgmt_rp_read_controller_cap *rp = (void *)buf;
+	u16 cap_len = 0;
 	u8 flags = 0;
+	u8 tx_power_range[2];
 
 	bt_dev_dbg(hdev, "sock %p", sk);
 
@@ -3740,23 +3741,33 @@  static int read_security_info(struct sock *sk, struct hci_dev *hdev,
 
 	flags |= 0x08;		/* Encryption key size enforcement (LE) */
 
-	sec_len = eir_append_data(rp->sec, sec_len, 0x01, &flags, 1);
+	cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_SEC_FLAGS,
+				  &flags, 1);
 
 	/* When the Read Simple Pairing Options command is supported, then
 	 * also max encryption key size information is provided.
 	 */
 	if (hdev->commands[41] & 0x08)
-		sec_len = eir_append_le16(rp->sec, sec_len, 0x02,
+		cap_len = eir_append_le16(rp->cap, cap_len,
+					  MGMT_CAP_MAX_ENC_KEY_SIZE,
 					  hdev->max_enc_key_size);
 
-	sec_len = eir_append_le16(rp->sec, sec_len, 0x03, SMP_MAX_ENC_KEY_SIZE);
+	cap_len = eir_append_le16(rp->cap, cap_len,
+				  MGMT_CAP_SMP_MAX_ENC_KEY_SIZE,
+				  SMP_MAX_ENC_KEY_SIZE);
+
+	/* Append the min/max LE tx power parameters */
+	memcpy(&tx_power_range[0], &hdev->min_le_tx_power, 1);
+	memcpy(&tx_power_range[1], &hdev->max_le_tx_power, 1);
+	cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_LE_TX_PWR,
+				  tx_power_range, 2);
 
-	rp->sec_len = cpu_to_le16(sec_len);
+	rp->cap_len = cpu_to_le16(cap_len);
 
 	hci_dev_unlock(hdev);
 
-	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_READ_SECURITY_INFO, 0,
-				 rp, sizeof(*rp) + sec_len);
+	return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_READ_CONTROLLER_CAP, 0,
+				 rp, sizeof(*rp) + cap_len);
 }
 
 #ifdef CONFIG_BT_FEATURE_DEBUG
@@ -8193,7 +8204,7 @@  static const struct hci_mgmt_handler mgmt_handlers[] = {
 	{ set_blocked_keys,	   MGMT_OP_SET_BLOCKED_KEYS_SIZE,
 						HCI_MGMT_VAR_LEN },
 	{ set_wideband_speech,	   MGMT_SETTING_SIZE },
-	{ read_security_info,      MGMT_READ_SECURITY_INFO_SIZE,
+	{ read_controller_cap,     MGMT_READ_CONTROLLER_CAP_SIZE,
 						HCI_MGMT_UNTRUSTED },
 	{ read_exp_features_info,  MGMT_READ_EXP_FEATURES_INFO_SIZE,
 						HCI_MGMT_UNTRUSTED |