| Message ID | 20240828181051.876598-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4e76e85acacd6258a3cb54f23f63442407e4e1f0 |
| Headers | show |
| Series | [v2,1/2] Revert "Bluetooth: MGMT/SMP: Fix address type when using SMP over BREDR/LE" | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (84>80): "[v2,1/2] Revert "Bluetooth: MGMT/SMP: Fix address type when using SMP over BREDR/LE"" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | warning | CheckSparse WARNING net/bluetooth/mgmt.c: In function ‘load_link_keys’:./include/net/bluetooth/bluetooth.h:276:10: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘__u8 *’ {aka ‘unsigned char *’} [-Wformat=] 276 | BT_WARN("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__) | ^~~~~~./include/net/bluetooth/bluetooth.h:262:35: note: in definition of macro ‘BT_WARN’ 262 | #define BT_WARN(fmt, ...) bt_warn(fmt "\n", ##__VA_ARGS__) | ^~~net/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’ 2858 | bt_dev_warn(hdev, | ^~~~~~~~~~~net/bluetooth/mgmt.c:2859:37: note: format string is defined here 2859 | "Invalid link address type %u for %pMR", | ~^ | | | unsigned int | %hhnnet/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’ 2858 | bt_dev_warn(hdev, | ^~~~~~~~~~~net/bluetooth/mgmt.c:2859:44: note: format string is defined here 2859 | "Invalid link address type %u for %pMR", | ~^ | | | void *net/bluetooth/mgmt.c: In function ‘load_link_keys’:./include/net/bluetooth/bluetooth.h:276:10: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘__u8 *’ {aka ‘unsigned char *’} [-Wformat=] 276 | BT_WARN("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__) | ^~~~~~./include/net/bluetooth/bluetooth.h:262:35: note: in definition of macro ‘BT_WARN’ 262 | #define BT_WARN(fmt, ...) bt_warn(fmt "\n", ##__VA_ARGS__) | ^~~net/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’ 2858 | bt_dev_warn(hdev, | ^~~~~~~~~~~net/bluetooth/mgmt.c:2859:37: note: format string is defined here 2859 | "Invalid link address type %u for %pMR", | ~^ | | | unsigned int | %hhnnet/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’ 2858 | bt_dev_warn(hdev, | ^~~~~~~~~~~net/bluetooth/mgmt.c:2859:44: note: format string is defined here 2859 | "Invalid link address type %u for %pMR", | ~^ | | | void * |
| tedd_an/CheckSparse | warning | CheckSparse WARNING net/bluetooth/mgmt.c: In function ‘load_link_keys’:net/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’net/bluetooth/mgmt.c:2859:37: note: format string is defined herenet/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’net/bluetooth/mgmt.c:2859:44: note: format string is defined herenet/bluetooth/mgmt.c: In function ‘load_link_keys’:net/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’net/bluetooth/mgmt.c:2859:37: note: format string is defined herenet/bluetooth/mgmt.c:2858:4: note: in expansion of macro ‘bt_dev_warn’net/bluetooth/mgmt.c:2859:44: note: format string is defined here |
Hello: This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Wed, 28 Aug 2024 14:10:50 -0400 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This reverts commit 59b047bc98084f8af2c41483e4d68a5adf2fa7f7 which > breaks compatibility with commands like: > > bluetoothd[46328]: @ MGMT Command: Load.. (0x0013) plen 74 {0x0001} [hci0] > Keys: 2 > BR/EDR Address: C0:DC:DA:A5:E5:47 (Samsung Electronics Co.,Ltd) > Key type: Authenticated key from P-256 (0x03) > Central: 0x00 > Encryption size: 16 > Diversifier[2]: 0000 > Randomizer[8]: 0000000000000000 > Key[16]: 6ed96089bd9765be2f2c971b0b95f624 > LE Address: D7:2A:DE:1E:73:A2 (Static) > Key type: Unauthenticated key from P-256 (0x02) > Central: 0x00 > Encryption size: 16 > Diversifier[2]: 0000 > Randomizer[8]: 0000000000000000 > Key[16]: 87dd2546ededda380ffcdc0a8faa4597 > @ MGMT Event: Command Status (0x0002) plen 3 {0x0001} [hci0] > Load Long Term Keys (0x0013) > Status: Invalid Parameters (0x0d) > > [...] Here is the summary with links: - [v2,1/2] Revert "Bluetooth: MGMT/SMP: Fix address type when using SMP over BREDR/LE" https://git.kernel.org/bluetooth/bluetooth-next/c/4e76e85acacd - [v2,2/2] Bluetooth: MGMT: Ignore keys being loaded with invalid type https://git.kernel.org/bluetooth/bluetooth-next/c/c858dc985d70 You are awesome, thank you!
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e449dba698f3..1a32e602630e 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -186,7 +186,6 @@ struct blocked_key { struct smp_csrk { bdaddr_t bdaddr; u8 bdaddr_type; - u8 link_type; u8 type; u8 val[16]; }; @@ -196,7 +195,6 @@ struct smp_ltk { struct rcu_head rcu; bdaddr_t bdaddr; u8 bdaddr_type; - u8 link_type; u8 authenticated; u8 type; u8 enc_size; @@ -211,7 +209,6 @@ struct smp_irk { bdaddr_t rpa; bdaddr_t bdaddr; u8 addr_type; - u8 link_type; u8 val[16]; }; @@ -219,8 +216,6 @@ struct link_key { struct list_head list; struct rcu_head rcu; bdaddr_t bdaddr; - u8 bdaddr_type; - u8 link_type; u8 type; u8 val[HCI_LINK_KEY_SIZE]; u8 pin_len; diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 4c20dbf92c71..240dd8cf7c7d 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -2833,8 +2833,7 @@ 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]; - /* Considering SMP over BREDR/LE, there is no need to check addr_type */ - if (key->type > 0x08) + if (key->addr.type != BDADDR_BREDR || key->type > 0x08) return mgmt_cmd_status(sk, hdev->id, MGMT_OP_LOAD_LINK_KEYS, MGMT_STATUS_INVALID_PARAMS); @@ -7089,7 +7088,6 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data, for (i = 0; i < irk_count; i++) { struct mgmt_irk_info *irk = &cp->irks[i]; - u8 addr_type = le_addr_type(irk->addr.type); if (hci_is_blocked_key(hdev, HCI_BLOCKED_KEY_TYPE_IRK, @@ -7099,12 +7097,8 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data, continue; } - /* When using SMP over BR/EDR, the addr type should be set to BREDR */ - if (irk->addr.type == BDADDR_BREDR) - addr_type = BDADDR_BREDR; - hci_add_irk(hdev, &irk->addr.bdaddr, - addr_type, irk->val, + le_addr_type(irk->addr.type), irk->val, BDADDR_ANY); } @@ -7185,7 +7179,6 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev, for (i = 0; i < key_count; i++) { struct mgmt_ltk_info *key = &cp->keys[i]; u8 type, authenticated; - u8 addr_type = le_addr_type(key->addr.type); if (hci_is_blocked_key(hdev, HCI_BLOCKED_KEY_TYPE_LTK, @@ -7220,12 +7213,8 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev, continue; } - /* When using SMP over BR/EDR, the addr type should be set to BREDR */ - if (key->addr.type == BDADDR_BREDR) - addr_type = BDADDR_BREDR; - hci_add_ltk(hdev, &key->addr.bdaddr, - addr_type, type, authenticated, + le_addr_type(key->addr.type), type, authenticated, key->val, key->enc_size, key->ediv, key->rand); } @@ -9519,7 +9508,7 @@ void mgmt_new_link_key(struct hci_dev *hdev, struct link_key *key, ev.store_hint = persistent; bacpy(&ev.key.addr.bdaddr, &key->bdaddr); - ev.key.addr.type = link_to_bdaddr(key->link_type, key->bdaddr_type); + ev.key.addr.type = BDADDR_BREDR; ev.key.type = key->type; memcpy(ev.key.val, key->val, HCI_LINK_KEY_SIZE); ev.key.pin_len = key->pin_len; @@ -9570,7 +9559,7 @@ void mgmt_new_ltk(struct hci_dev *hdev, struct smp_ltk *key, bool persistent) ev.store_hint = persistent; bacpy(&ev.key.addr.bdaddr, &key->bdaddr); - ev.key.addr.type = link_to_bdaddr(key->link_type, key->bdaddr_type); + ev.key.addr.type = link_to_bdaddr(LE_LINK, key->bdaddr_type); ev.key.type = mgmt_ltk_type(key); ev.key.enc_size = key->enc_size; ev.key.ediv = key->ediv; @@ -9599,7 +9588,7 @@ void mgmt_new_irk(struct hci_dev *hdev, struct smp_irk *irk, bool persistent) bacpy(&ev.rpa, &irk->rpa); bacpy(&ev.irk.addr.bdaddr, &irk->bdaddr); - ev.irk.addr.type = link_to_bdaddr(irk->link_type, irk->addr_type); + ev.irk.addr.type = link_to_bdaddr(LE_LINK, irk->addr_type); memcpy(ev.irk.val, irk->val, sizeof(irk->val)); mgmt_event(MGMT_EV_NEW_IRK, hdev, &ev, sizeof(ev), NULL); @@ -9628,7 +9617,7 @@ void mgmt_new_csrk(struct hci_dev *hdev, struct smp_csrk *csrk, ev.store_hint = persistent; bacpy(&ev.key.addr.bdaddr, &csrk->bdaddr); - ev.key.addr.type = link_to_bdaddr(csrk->link_type, csrk->bdaddr_type); + ev.key.addr.type = link_to_bdaddr(LE_LINK, csrk->bdaddr_type); ev.key.type = csrk->type; memcpy(ev.key.val, csrk->val, sizeof(csrk->val)); diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 4f9fdf400584..8b9724fd752a 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -1060,7 +1060,6 @@ static void smp_notify_keys(struct l2cap_conn *conn) } if (smp->remote_irk) { - smp->remote_irk->link_type = hcon->type; mgmt_new_irk(hdev, smp->remote_irk, persistent); /* Now that user space can be considered to know the @@ -1080,28 +1079,24 @@ static void smp_notify_keys(struct l2cap_conn *conn) } if (smp->csrk) { - smp->csrk->link_type = hcon->type; smp->csrk->bdaddr_type = hcon->dst_type; bacpy(&smp->csrk->bdaddr, &hcon->dst); mgmt_new_csrk(hdev, smp->csrk, persistent); } if (smp->responder_csrk) { - smp->responder_csrk->link_type = hcon->type; smp->responder_csrk->bdaddr_type = hcon->dst_type; bacpy(&smp->responder_csrk->bdaddr, &hcon->dst); mgmt_new_csrk(hdev, smp->responder_csrk, persistent); } if (smp->ltk) { - smp->ltk->link_type = hcon->type; smp->ltk->bdaddr_type = hcon->dst_type; bacpy(&smp->ltk->bdaddr, &hcon->dst); mgmt_new_ltk(hdev, smp->ltk, persistent); } if (smp->responder_ltk) { - smp->responder_ltk->link_type = hcon->type; smp->responder_ltk->bdaddr_type = hcon->dst_type; bacpy(&smp->responder_ltk->bdaddr, &hcon->dst); mgmt_new_ltk(hdev, smp->responder_ltk, persistent); @@ -1121,8 +1116,6 @@ static void smp_notify_keys(struct l2cap_conn *conn) key = hci_add_link_key(hdev, smp->conn->hcon, &hcon->dst, smp->link_key, type, 0, &persistent); if (key) { - key->link_type = hcon->type; - key->bdaddr_type = hcon->dst_type; mgmt_new_link_key(hdev, key, persistent); /* Don't keep debug keys around if the relevant