2015-08-19 14:25:30

by Nicholas Krause

[permalink] [raw]
Subject: [PATCH] bluetooth:Fix NULL pointer deference issue in the function load_irks

This fixes a possible NULL pointer deference issue in the function
load_irks if the call to the function hci_add_link fails and returns
NULL by instead checking if this return value occurs before returning
-ENOMEM to the function load_irks caller as the function hci_add_link
only fails due to a memory allocation error therfore due to this we
only need to return the error -ENOMEM directly to the function load_irks

Signed-off-by: Nicholas Krause <[email protected]>
---
net/bluetooth/mgmt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 92720f3..e286d63 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5602,8 +5602,9 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
else
addr_type = ADDR_LE_DEV_RANDOM;

- hci_add_irk(hdev, &irk->addr.bdaddr, addr_type, irk->val,
- BDADDR_ANY);
+ if (!hci_add_irk(hdev, &irk->addr.bdaddr, addr_type, irk->val,
+ BDADDR_ANY))
+ return -ENOMEM;
}

hci_dev_set_flag(hdev, HCI_RPA_RESOLVING);
--
2.1.4


2015-08-24 09:54:26

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH] bluetooth:Fix NULL pointer deference issue in the function load_irks

Hi Nicholas,

(dropping unneeded recipients from CC - in the future linux-bluetooth
should be enough for patches like this)

On Wed, Aug 19, 2015, Nicholas Krause wrote:
> This fixes a possible NULL pointer deference issue in the function
> load_irks if the call to the function hci_add_link fails and returns
> NULL

I don't see what NULL dereference this is fixing. The load_irk()
function doesn't do anything with the returned pointer from
hci_add_irk().

> net/bluetooth/mgmt.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 92720f3..e286d63 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -5602,8 +5602,9 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
> else
> addr_type = ADDR_LE_DEV_RANDOM;
>
> - hci_add_irk(hdev, &irk->addr.bdaddr, addr_type, irk->val,
> - BDADDR_ANY);
> + if (!hci_add_irk(hdev, &irk->addr.bdaddr, addr_type, irk->val,
> + BDADDR_ANY))
> + return -ENOMEM;

This could result in a partial IRK list being stored in the kernel, i.e.
you should at least make a call to hci_smp_irks_clear() before returning
a failure.

Johan