2019-03-27 04:35:14

by K, SpoorthiX

[permalink] [raw]
Subject: [PATCH] Add Support for LE Ping feature

From: Spoorthi Ravishankar Koppad <[email protected]>

Changes made to add HCI Write Authenticated Payload timeout
command for LE Ping feature.
As per the Core Specification 5.0 Volume 2 Part E Section 7.3.94,
the following code changes implements
HCI Write Authenticated Payload timeout command for LE Ping feature.

Signed-off-by: Spoorthi Ravishankar Koppad <[email protected]>
---
include/net/bluetooth/hci.h | 10 +++++++++
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_conn.c | 1 +
net/bluetooth/hci_core.c | 1 +
net/bluetooth/hci_debugfs.c | 31 ++++++++++++++++++++++++++++
net/bluetooth/hci_event.c | 44 ++++++++++++++++++++++++++++++++++++++++
6 files changed, 89 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index fbba43e..769a007 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1142,6 +1142,16 @@ struct hci_cp_write_sc_support {
__u8 support;
} __packed;

+#define HCI_OP_WRITE_AUTH_PAYLOAD_TO 0x0c7c
+struct hci_cp_write_auth_payload_to {
+ __le16 handle;
+ __u16 timeout;
+} __packed;
+struct hci_rp_write_auth_payload_to {
+ __u8 status;
+ __le16 handle;
+} __packed;
+
#define HCI_OP_READ_LOCAL_OOB_EXT_DATA 0x0c7d
struct hci_rp_read_local_oob_ext_data {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 094e61e..ce0c7a2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -421,6 +421,7 @@ struct hci_dev {
__u32 rpa_timeout;
struct delayed_work rpa_expired;
bdaddr_t rpa;
+ __u16 auth_payload_timeout;

#if IS_ENABLED(CONFIG_BT_LEDS)
struct led_trigger *power_led;
@@ -500,6 +501,7 @@ struct hci_conn {
__u8 remote_id;

unsigned int sent;
+ bool auth_payload_timeout_set;

struct sk_buff_head data_q;
struct list_head chan_list;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index bd4978c..cf036b8 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -516,6 +516,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
conn->rssi = HCI_RSSI_INVALID;
conn->tx_power = HCI_TX_POWER_INVALID;
conn->max_tx_power = HCI_TX_POWER_INVALID;
+ conn->auth_payload_timeout_set = false;

set_bit(HCI_CONN_POWER_SAVE, &conn->flags);
conn->disc_timeout = HCI_DISCONN_TIMEOUT;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d6b2540..29f524d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3193,6 +3193,7 @@ struct hci_dev *hci_alloc_dev(void)
hdev->le_max_tx_time = 0x0148;
hdev->le_max_rx_len = 0x001b;
hdev->le_max_rx_time = 0x0148;
+ hdev->auth_payload_timeout = 0x0bb8;
hdev->le_max_key_size = SMP_MAX_ENC_KEY_SIZE;
hdev->le_min_key_size = SMP_MIN_ENC_KEY_SIZE;
hdev->le_tx_def_phys = HCI_LE_SET_PHY_1M;
diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 51f5b1e..f7ab4d6 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -941,6 +941,35 @@ static int adv_max_interval_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(adv_max_interval_fops, adv_max_interval_get,
adv_max_interval_set, "%llu\n");

+static int auth_payload_timeout_set(void *data, u64 val)
+{
+ struct hci_dev *hdev = data;
+
+ if (val < 0x0001 || val > 0xffff)
+ return -EINVAL;
+
+ hci_dev_lock(hdev);
+ hdev->auth_payload_timeout = val;
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+static int auth_payload_timeout_get(void *data, u64 *val)
+{
+ struct hci_dev *hdev = data;
+
+ hci_dev_lock(hdev);
+ *val = hdev->auth_payload_timeout;
+ hci_dev_unlock(hdev);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(auth_payload_timeout_fops,
+ auth_payload_timeout_get,
+ auth_payload_timeout_set, "%llu\n");
+
DEFINE_QUIRK_ATTRIBUTE(quirk_strict_duplicate_filter,
HCI_QUIRK_STRICT_DUPLICATE_FILTER);
DEFINE_QUIRK_ATTRIBUTE(quirk_simultaneous_discovery,
@@ -992,6 +1021,8 @@ void hci_debugfs_create_le(struct hci_dev *hdev)
&adv_min_interval_fops);
debugfs_create_file("adv_max_interval", 0644, hdev->debugfs, hdev,
&adv_max_interval_fops);
+ debugfs_create_file("auth_payload_timeout", 0644, hdev->debugfs, hdev,
+ &auth_payload_timeout_fops);
debugfs_create_u16("discov_interleaved_timeout", 0644, hdev->debugfs,
&hdev->discov_interleaved_timeout);

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 609fd68..94a8e58 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -564,6 +564,25 @@ static void hci_cc_read_local_version(struct hci_dev *hdev, struct sk_buff *skb)
}
}

+static void hci_cc_write_auth_payload_timeout(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_write_auth_payload_to *rp = (void *)skb->data;
+ struct hci_conn *conn;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ hci_dev_lock(hdev);
+ conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(rp->handle));
+
+ if (conn)
+ conn->auth_payload_timeout_set = true;
+ hci_dev_unlock(hdev);
+}
+
static void hci_cc_read_local_commands(struct hci_dev *hdev,
struct sk_buff *skb)
{
@@ -2974,6 +2993,27 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)

goto unlock;
}
+ /* Set the default Authenticated Payload Timeout after
+ * an LE Link is established. As per Core Spec v5.0, Vol 2, Part B
+ * Section 3.3, the HCI command WRITE_AUTH_PAYLOAD_TIMEOUT should be
+ * sent when the link is active and Encryption is enabled, the conn
+ * type can be either LE or ACL and controller must support LMP Ping.
+ * Ensure for AES-CCM encryption as well.
+ */
+ if ((conn->type == LE_LINK || conn->type == ACL_LINK) &
+ lmp_ping_capable(hdev) &&
+ test_bit(HCI_CONN_ENCRYPT, &conn->flags) &&
+ test_bit(HCI_CONN_AES_CCM, &conn->flags)) {
+ struct hci_cp_write_auth_payload_to cp;
+
+ cp.handle = cpu_to_le16(conn->handle);
+ cp.timeout = cpu_to_le16(hdev->auth_payload_timeout);
+ hci_send_cmd(conn->hdev, HCI_OP_WRITE_AUTH_PAYLOAD_TO,
+ sizeof(cp), &cp);
+ } else {
+ conn->state = BT_CONNECTED;
+ hci_connect_cfm(conn, ev->status);
+ }

notify:
if (conn->state == BT_CONFIG) {
@@ -3170,6 +3210,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_write_sc_support(hdev, skb);
break;

+ case HCI_OP_WRITE_AUTH_PAYLOAD_TO:
+ hci_cc_write_auth_payload_timeout(hdev, skb);
+ break;
+
case HCI_OP_READ_LOCAL_VERSION:
hci_cc_read_local_version(hdev, skb);
break;
--
1.9.1



2019-03-27 13:39:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] Add Support for LE Ping feature

Hi SpoorthiX,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on v5.1-rc2 next-20190327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/SpoorthiX-K/Add-Support-for-LE-Ping-feature/20190327-182038
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'


sparse warnings: (new ones prefixed by >>)

>> net/bluetooth/hci_event.c:3010:28: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [unsigned] [assigned] [usertype] timeout @@ got igned] [assigned] [usertype] timeout @@
net/bluetooth/hci_event.c:3010:28: expected unsigned short [unsigned] [assigned] [usertype] timeout
net/bluetooth/hci_event.c:3010:28: got restricted __le16 [usertype] <noident>
include/linux/overflow.h:285:13: sparse: undefined identifier '__builtin_mul_overflow'
include/linux/overflow.h:285:13: sparse: incorrect type in conditional
include/linux/overflow.h:285:13: got void
include/linux/overflow.h:287:13: sparse: undefined identifier '__builtin_add_overflow'
include/linux/overflow.h:287:13: sparse: incorrect type in conditional
include/linux/overflow.h:287:13: got void
include/linux/overflow.h:285:13: sparse: not a function <noident>
include/linux/overflow.h:285:13: sparse: incorrect type in conditional
include/linux/overflow.h:285:13: got void
include/linux/overflow.h:287:13: sparse: not a function <noident>
include/linux/overflow.h:287:13: sparse: incorrect type in conditional
include/linux/overflow.h:287:13: got void
include/linux/overflow.h:285:13: sparse: call with no type!
include/linux/overflow.h:287:13: sparse: call with no type!

vim +3010 net/bluetooth/hci_event.c

2904
2905 static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
2906 {
2907 struct hci_ev_encrypt_change *ev = (void *) skb->data;
2908 struct hci_conn *conn;
2909
2910 BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
2911
2912 hci_dev_lock(hdev);
2913
2914 conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(ev->handle));
2915 if (!conn)
2916 goto unlock;
2917
2918 if (!ev->status) {
2919 if (ev->encrypt) {
2920 /* Encryption implies authentication */
2921 set_bit(HCI_CONN_AUTH, &conn->flags);
2922 set_bit(HCI_CONN_ENCRYPT, &conn->flags);
2923 conn->sec_level = conn->pending_sec_level;
2924
2925 /* P-256 authentication key implies FIPS */
2926 if (conn->key_type == HCI_LK_AUTH_COMBINATION_P256)
2927 set_bit(HCI_CONN_FIPS, &conn->flags);
2928
2929 if ((conn->type == ACL_LINK && ev->encrypt == 0x02) ||
2930 conn->type == LE_LINK)
2931 set_bit(HCI_CONN_AES_CCM, &conn->flags);
2932 } else {
2933 clear_bit(HCI_CONN_ENCRYPT, &conn->flags);
2934 clear_bit(HCI_CONN_AES_CCM, &conn->flags);
2935 }
2936 }
2937
2938 /* We should disregard the current RPA and generate a new one
2939 * whenever the encryption procedure fails.
2940 */
2941 if (ev->status && conn->type == LE_LINK) {
2942 hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
2943 hci_adv_instances_set_rpa_expired(hdev, true);
2944 }
2945
2946 clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
2947
2948 if (ev->status && conn->state == BT_CONNECTED) {
2949 if (ev->status == HCI_ERROR_PIN_OR_KEY_MISSING)
2950 set_bit(HCI_CONN_AUTH_FAILURE, &conn->flags);
2951
2952 hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE);
2953 hci_conn_drop(conn);
2954 goto unlock;
2955 }
2956
2957 /* In Secure Connections Only mode, do not allow any connections
2958 * that are not encrypted with AES-CCM using a P-256 authenticated
2959 * combination key.
2960 */
2961 if (hci_dev_test_flag(hdev, HCI_SC_ONLY) &&
2962 (!test_bit(HCI_CONN_AES_CCM, &conn->flags) ||
2963 conn->key_type != HCI_LK_AUTH_COMBINATION_P256)) {
2964 hci_connect_cfm(conn, HCI_ERROR_AUTH_FAILURE);
2965 hci_conn_drop(conn);
2966 goto unlock;
2967 }
2968
2969 /* Try reading the encryption key size for encrypted ACL links */
2970 if (!ev->status && ev->encrypt && conn->type == ACL_LINK) {
2971 struct hci_cp_read_enc_key_size cp;
2972 struct hci_request req;
2973
2974 /* Only send HCI_Read_Encryption_Key_Size if the
2975 * controller really supports it. If it doesn't, assume
2976 * the default size (16).
2977 */
2978 if (!(hdev->commands[20] & 0x10)) {
2979 conn->enc_key_size = HCI_LINK_KEY_SIZE;
2980 goto notify;
2981 }
2982
2983 hci_req_init(&req, hdev);
2984
2985 cp.handle = cpu_to_le16(conn->handle);
2986 hci_req_add(&req, HCI_OP_READ_ENC_KEY_SIZE, sizeof(cp), &cp);
2987
2988 if (hci_req_run_skb(&req, read_enc_key_size_complete)) {
2989 bt_dev_err(hdev, "sending read key size failed");
2990 conn->enc_key_size = HCI_LINK_KEY_SIZE;
2991 goto notify;
2992 }
2993
2994 goto unlock;
2995 }
2996 /* Set the default Authenticated Payload Timeout after
2997 * an LE Link is established. As per Core Spec v5.0, Vol 2, Part B
2998 * Section 3.3, the HCI command WRITE_AUTH_PAYLOAD_TIMEOUT should be
2999 * sent when the link is active and Encryption is enabled, the conn
3000 * type can be either LE or ACL and controller must support LMP Ping.
3001 * Ensure for AES-CCM encryption as well.
3002 */
3003 if ((conn->type == LE_LINK || conn->type == ACL_LINK) &
3004 lmp_ping_capable(hdev) &&
3005 test_bit(HCI_CONN_ENCRYPT, &conn->flags) &&
3006 test_bit(HCI_CONN_AES_CCM, &conn->flags)) {
3007 struct hci_cp_write_auth_payload_to cp;
3008
3009 cp.handle = cpu_to_le16(conn->handle);
> 3010 cp.timeout = cpu_to_le16(hdev->auth_payload_timeout);
3011 hci_send_cmd(conn->hdev, HCI_OP_WRITE_AUTH_PAYLOAD_TO,
3012 sizeof(cp), &cp);
3013 } else {
3014 conn->state = BT_CONNECTED;
3015 hci_connect_cfm(conn, ev->status);
3016 }
3017
3018 notify:
3019 if (conn->state == BT_CONFIG) {
3020 if (!ev->status)
3021 conn->state = BT_CONNECTED;
3022
3023 hci_connect_cfm(conn, ev->status);
3024 hci_conn_drop(conn);
3025 } else
3026 hci_encrypt_cfm(conn, ev->status, ev->encrypt);
3027
3028 unlock:
3029 hci_dev_unlock(hdev);
3030 }
3031

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

2019-03-27 17:29:54

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] Add Support for LE Ping feature

Hi Spoorthi,

On 27 Mar 2019, at 5.43, SpoorthiX K <[email protected]> wrote:
> +#define HCI_OP_WRITE_AUTH_PAYLOAD_TO 0x0c7c
> +struct hci_cp_write_auth_payload_to {
> + __le16 handle;
> + __u16 timeout;

As Marcel (and now also the build test robot) pointed out this should be __le16.

> #if IS_ENABLED(CONFIG_BT_LEDS)
> struct led_trigger *power_led;
> @@ -500,6 +501,7 @@ struct hci_conn {
> __u8 remote_id;
>
> unsigned int sent;
> + bool auth_payload_timeout_set;

Would it make sense to add this to the flags instead of increasing the hci_conn struct size?

> + /* Set the default Authenticated Payload Timeout after
> + * an LE Link is established.

The test further below also includes BR/EDR links, i.e. the above comment doesn’t match that.

> + * Section 3.3, the HCI command WRITE_AUTH_PAYLOAD_TIMEOUT should be
> + * sent when the link is active and Encryption is enabled, the conn
> + * type can be either LE or ACL and controller must support LMP Ping.
> + * Ensure for AES-CCM encryption as well.
> + */
> + if ((conn->type == LE_LINK || conn->type == ACL_LINK) &

That ‘&’ the end should probably be a ‘&&’, shouldn’t it?

> + } else {
> + conn->state = BT_CONNECTED;
> + hci_connect_cfm(conn, ev->status);
> + }

Why is this being added? Such a transition to BT_CONNECTED state at this point in the code didn’t exist previously, and this doesn’t seem to be related to the new feature.

Johan