Re: [PATCH] bluetooth: Fix possible NULL dereference

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

>>> conn might be NULL and would be dereferenced in conn_set_key()
>>> ---
>>> net/bluetooth/hci_event.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>> index bd0a801..95f8057 100644
>>> --- a/net/bluetooth/hci_event.c
>>> +++ b/net/bluetooth/hci_event.c
>>> @@ -3312,7 +3312,7 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff *skb)
>>> 	/* Update connection information since adding the key will have
>>> 	 * fixed up the type in the case of changed combination keys.
>>> 	 */
>>> -	if (ev->key_type == HCI_LK_CHANGED_COMBINATION)
>>> +	if (conn && ev->key_type == HCI_LK_CHANGED_COMBINATION)
>>> 		conn_set_key(conn, key->type, key->pin_len);
>>> 
>>> 	mgmt_new_link_key(hdev, key, persistent);
>> 
>> the more important question is why we are considering link keys from
>> the controller for a device that we do not have a hci_conn object for.
>> 
>> If this is for security mode 3, then we should handle security mode 3
>> and then bail out of this function. Since security mode 3 means that
>> we never get SSP or SC based keys. It is legacy pairing and does not
>> need any special handling for debug keys or changed combination keys.
> 
> The existence of a hci_conn object in this case is not dependent on
> whether we're in security mode 3 or not. The hci_conn should have either
> way been created when we initiated the connection or received a
> connection request (only speciality with sec mode 3 is when the conn
> complete finally comes). So this particular fix is really only for a
> coverity warning as far as I see (and some very broken controllers).
> 
> As for security mode 3 connections we would still want to make a note of
> the key type and PIN length.

then please look through the whole function. We have if (conn) checks sprinkled all over it. This needs to be re-written to make it clean.

All the handling for SSP or SC should not even be considered when we do not have hci_conn object. I would say, do the minimum if we do not have hci_conn object and bail out. And let the SSP and SC pieces be executed otherwise.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux