* [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).