* [PATCH] Bluetooth: mgmt: Fix possible NULL dereference
@ 2012-07-19 9:49 Andrei Emeltchenko
2012-07-19 10:23 ` Johan Hedberg
0 siblings, 1 reply; 4+ messages in thread
From: Andrei Emeltchenko @ 2012-07-19 9:49 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
hdev might be dereferenced in handler->func functions.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/mgmt.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2a0f695..48a83c9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2801,14 +2801,15 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
goto done;
}
- if (hdev)
+ if (hdev) {
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: Fix possible NULL dereference
2012-07-19 9:49 [PATCH] Bluetooth: mgmt: Fix possible NULL dereference Andrei Emeltchenko
@ 2012-07-19 10:23 ` Johan Hedberg
2012-07-19 10:38 ` Andrei Emeltchenko
0 siblings, 1 reply; 4+ messages in thread
From: Johan Hedberg @ 2012-07-19 10:23 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
Hi Andrei,
On Thu, Jul 19, 2012, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> hdev might be dereferenced in handler->func functions.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> net/bluetooth/mgmt.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 2a0f695..48a83c9 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2801,14 +2801,15 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> goto done;
> }
>
> - if (hdev)
> + if (hdev) {
> 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;
Nack.
hdev is supposed to be NULL for some of the commands/handlers
(read_version, read_index_list, etc). With your change these mgmt
commands couldn't be called anymore. There's also a check for this
higher up in the mgmt_control function to ensure that hdev is not NULL
for other handlers:
if ((hdev && opcode < MGMT_OP_READ_INFO) ||
(!hdev && opcode >= MGMT_OP_READ_INFO)) {
err = cmd_status(sk, index, opcode,
MGMT_STATUS_INVALID_INDEX);
goto done;
}
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: Fix possible NULL dereference
2012-07-19 10:23 ` Johan Hedberg
@ 2012-07-19 10:38 ` Andrei Emeltchenko
2012-07-19 11:15 ` Johan Hedberg
0 siblings, 1 reply; 4+ messages in thread
From: Andrei Emeltchenko @ 2012-07-19 10:38 UTC (permalink / raw)
To: linux-bluetooth
Hi Johan,
On Thu, Jul 19, 2012 at 01:23:06PM +0300, Johan Hedberg wrote:
> Hi Andrei,
>
> On Thu, Jul 19, 2012, Andrei Emeltchenko wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > hdev might be dereferenced in handler->func functions.
> >
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > ---
> > net/bluetooth/mgmt.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 2a0f695..48a83c9 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -2801,14 +2801,15 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> > goto done;
> > }
> >
> > - if (hdev)
> > + if (hdev) {
> > 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;
>
> Nack.
>
> hdev is supposed to be NULL for some of the commands/handlers
> (read_version, read_index_list, etc). With your change these mgmt
> commands couldn't be called anymore. There's also a check for this
> higher up in the mgmt_control function to ensure that hdev is not NULL
> for other handlers:
OK, though I do not like that division by command name.
>
> if ((hdev && opcode < MGMT_OP_READ_INFO) ||
> (!hdev && opcode >= MGMT_OP_READ_INFO)) {
> err = cmd_status(sk, index, opcode,
> MGMT_STATUS_INVALID_INDEX);
> goto done;
> }
>
Best regards
Andrei Emeltchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: Fix possible NULL dereference
2012-07-19 10:38 ` Andrei Emeltchenko
@ 2012-07-19 11:15 ` Johan Hedberg
0 siblings, 0 replies; 4+ messages in thread
From: Johan Hedberg @ 2012-07-19 11:15 UTC (permalink / raw)
To: Andrei Emeltchenko, linux-bluetooth
Hi Andrei,
On Thu, Jul 19, 2012, Andrei Emeltchenko wrote:
> On Thu, Jul 19, 2012 at 01:23:06PM +0300, Johan Hedberg wrote:
> > Hi Andrei,
> >
> > On Thu, Jul 19, 2012, Andrei Emeltchenko wrote:
> > > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > >
> > > hdev might be dereferenced in handler->func functions.
> > >
> > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > ---
> > > net/bluetooth/mgmt.c | 11 ++++++-----
> > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > > index 2a0f695..48a83c9 100644
> > > --- a/net/bluetooth/mgmt.c
> > > +++ b/net/bluetooth/mgmt.c
> > > @@ -2801,14 +2801,15 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> > > goto done;
> > > }
> > >
> > > - if (hdev)
> > > + if (hdev) {
> > > 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;
> >
> > Nack.
> >
> > hdev is supposed to be NULL for some of the commands/handlers
> > (read_version, read_index_list, etc). With your change these mgmt
> > commands couldn't be called anymore. There's also a check for this
> > higher up in the mgmt_control function to ensure that hdev is not NULL
> > for other handlers:
>
> OK, though I do not like that division by command name.
I was originally thinking of having a boolean flag for that in the
look-up table but concluded that it's a bit overkill (not to mention
that it breaks the clean formatting) since only the first few commands
are the ones that don't take a valid controller index.
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-07-19 11:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-19 9:49 [PATCH] Bluetooth: mgmt: Fix possible NULL dereference Andrei Emeltchenko
2012-07-19 10:23 ` Johan Hedberg
2012-07-19 10:38 ` Andrei Emeltchenko
2012-07-19 11:15 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).