[PATCH] Bluetooth: mgmt: Fix dereference after NULL check

[PATCH] Bluetooth: mgmt: Fix dereference after NULL check

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

hdev might be NULL, so we need to avoid possible NULL derefernce in
handler->func functions.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/mgmt.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b127b88..be3182c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2738,7 +2738,6 @@ static const struct mgmt_handler {
 int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
 {
 	void *buf;
-	u8 *cp;
 	struct mgmt_hdr *hdr;
 	u16 opcode, index, len;
 	struct hci_dev *hdev = NULL;
@@ -2802,14 +2801,17 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
 		goto done;
 	}
 
-	if (hdev)
+	if (hdev) {
+		u8 *cp;
+
 		mgmt_init_hdev(sk, hdev);
 
-	cp = buf + sizeof(*hdr);
+		cp = buf + sizeof(*hdr);
 
-	err = handler->func(sk, hdev, cp, len);
-	if (err < 0)
-		goto done;
+		err = handler->func(sk, hdev, cp, len);
+		if (err < 0)
+			goto done;
+	}
 
 	err = msglen;
 
-- 
1.7.9.5

Re: [PATCH] Bluetooth: mgmt: Fix dereference after NULL check

[Johan Hedberg]

Hi Andrei,

On Thu, Oct 04, 2012, Andrei Emeltchenko wrote:
> hdev might be NULL, so we need to avoid possible NULL derefernce in
> handler->func functions.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/mgmt.c |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Nack. Some of the handlers actually should be getting a NULL hdev and
there's code higher up in the function you're changing that ensures that
NULL is allowed for those handlers but not for any others (which should
safely be able to skip NULL checks).

Johan

Re: [PATCH] Bluetooth: mgmt: Fix dereference after NULL check

[Andrei Emeltchenko]

Hi Johan,

On Thu, Oct 04, 2012 at 03:17:02PM +0300, Johan Hedberg wrote:
> Hi Andrei,
> 
> On Thu, Oct 04, 2012, Andrei Emeltchenko wrote:
> > hdev might be NULL, so we need to avoid possible NULL derefernce in
> > handler->func functions.
> > 
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > ---
> >  net/bluetooth/mgmt.c |   14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> Nack. Some of the handlers actually should be getting a NULL hdev and
> there's code higher up in the function you're changing that ensures that
> NULL is allowed for those handlers but not for any others (which should
> safely be able to skip NULL checks).

True, BTW: this would be nice to put this explanation as comment there.

Best regards 
Andrei Emeltchenko