Re: [PATCH 2/4] Bluetooth: 6lowpan: fix BDADDR_LE vs ADDR_LE_DEV address type confusion

[Paul Menzel <pmenzel@molgen.mpg.de>]

[Remove bouncing jukka.rissanen@linux.intel.com]

Am 01.11.25 um 17:24 schrieb Paul Menzel:
> Dear Pauli,
> 
> 
> Thank you for your patch.
> 
> Am 01.11.25 um 13:09 schrieb Pauli Virtanen:
>> Bluetooth 6lowpan.c confuses BDADDR_LE and ADDR_LE_DEV address types,
>> e.g. debugfs "connect" command takes the former, and "disconnect" and
>> "connect" to already connected device take the latter.  This is due to
>> using same value both for l2cap_chan_connect and hci_conn_hash_lookup_le
>> which take different dst_type values.
>>
>> Fix address type passed to hci_conn_hash_lookup_le().
>>
>> Retain the debugfs API difference between "connect" and "disconnect"
>> commands since it's been like this since 2015 and nobody apparently
>> complained.
>>
>> Fixes: f5ad4ffceba0 ("Bluetooth: 6lowpan: Use hci_conn_hash_lookup_le() when possible")
>> Signed-off-by: Pauli Virtanen <pav@iki.fi>
>> ---
>>   net/bluetooth/6lowpan.c | 28 ++++++++++++++++++++++++----
>>   1 file changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> index f1d29fa4b411..0d8c2e2e9a6c 100644
>> --- a/net/bluetooth/6lowpan.c
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -957,10 +957,11 @@ static struct l2cap_chan *bt_6lowpan_listen(void)
>>   }
>>   static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
>> -              struct l2cap_conn **conn)
>> +              struct l2cap_conn **conn, bool disconnect)
>>   {
>>       struct hci_conn *hcon;
>>       struct hci_dev *hdev;
>> +    int le_addr_type;
>>       int n;
>>       n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
>> @@ -971,13 +972,32 @@ static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
>>       if (n < 7)
>>           return -EINVAL;
>> +    if (disconnect) {
>> +        /* The "disconnect" debugfs command has used different address
>> +         * type constants than "connect" since 2015. Let's retain that
>> +         * for now even though it's obviously buggy...
>> +         */
>> +        *addr_type += 1;
>> +    }
>> +
>> +    switch (*addr_type) {
>> +    case BDADDR_LE_PUBLIC:
>> +        le_addr_type = ADDR_LE_DEV_PUBLIC;
>> +        break;
>> +    case BDADDR_LE_RANDOM:
>> +        le_addr_type = ADDR_LE_DEV_RANDOM;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>>       /* The LE_PUBLIC address type is ignored because of BDADDR_ANY */
>>       hdev = hci_get_route(addr, BDADDR_ANY, BDADDR_LE_PUBLIC);
>>       if (!hdev)
>>           return -ENOENT;
>>       hci_dev_lock(hdev);
>> -    hcon = hci_conn_hash_lookup_le(hdev, addr, *addr_type);
>> +    hcon = hci_conn_hash_lookup_le(hdev, addr, le_addr_type);
>>       hci_dev_unlock(hdev);
>>       hci_dev_put(hdev);
>> @@ -1104,7 +1124,7 @@ static ssize_t lowpan_control_write(struct file *fp,
>>       buf[buf_size] = '\0';
>>       if (memcmp(buf, "connect ", 8) == 0) {
>> -        ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn);
>> +        ret = get_l2cap_conn(&buf[8], &addr, &addr_type, &conn, false);
>>           if (ret == -EINVAL)
>>               return ret;
>> @@ -1141,7 +1161,7 @@ static ssize_t lowpan_control_write(struct file 
>> *fp,
>>       }
>>       if (memcmp(buf, "disconnect ", 11) == 0) {
>> -        ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn);
>> +        ret = get_l2cap_conn(&buf[11], &addr, &addr_type, &conn, true);
>>           if (ret < 0)
>>               return ret;
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul