All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: hci_conn: fix potential double free in le_scan_cleanup()
@ 2022-05-26  9:49 Jianglei Nie
  2022-05-26 11:24 ` Paolo Abeni
  0 siblings, 1 reply; 3+ messages in thread
From: Jianglei Nie @ 2022-05-26  9:49 UTC (permalink / raw)
  To: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni
  Cc: linux-bluetooth, netdev, linux-kernel, Jianglei Nie

When "c == conn" is true, hci_conn_cleanup() is called. The
hci_conn_cleanup() calls hci_dev_put() and hci_conn_put() in
its function implementation. hci_dev_put() and hci_conn_put()
will free the relevant resource if the reference count reaches
zero, which may lead to a double free when hci_dev_put() and
hci_conn_put() are called again.

We should add a return to this function after hci_conn_cleanup()
is called.

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
 net/bluetooth/hci_conn.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fe803bee419a..7b3e91eb9fa3 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -166,6 +166,7 @@ static void le_scan_cleanup(struct work_struct *work)
 	if (c == conn) {
 		hci_connect_le_scan_cleanup(conn);
 		hci_conn_cleanup(conn);
+		return;
 	}
 
 	hci_dev_unlock(hdev);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Bluetooth: hci_conn: fix potential double free in le_scan_cleanup()
  2022-05-26  9:49 [PATCH] Bluetooth: hci_conn: fix potential double free in le_scan_cleanup() Jianglei Nie
@ 2022-05-26 11:24 ` Paolo Abeni
  2022-05-26 20:11   ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2022-05-26 11:24 UTC (permalink / raw)
  To: Jianglei Nie, marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba
  Cc: linux-bluetooth, netdev, linux-kernel

On Thu, 2022-05-26 at 17:49 +0800, Jianglei Nie wrote:
> When "c == conn" is true, hci_conn_cleanup() is called. The
> hci_conn_cleanup() calls hci_dev_put() and hci_conn_put() in
> its function implementation. hci_dev_put() and hci_conn_put()
> will free the relevant resource if the reference count reaches
> zero, which may lead to a double free when hci_dev_put() and
> hci_conn_put() are called again.
> 
> We should add a return to this function after hci_conn_cleanup()
> is called.
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>  net/bluetooth/hci_conn.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index fe803bee419a..7b3e91eb9fa3 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -166,6 +166,7 @@ static void le_scan_cleanup(struct work_struct *work)
>  	if (c == conn) {
>  		hci_connect_le_scan_cleanup(conn);
>  		hci_conn_cleanup(conn);
> +		return;

This looks not correct. At very least you should release the
hci_dev_lock.

Cheers,

Paolo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Bluetooth: hci_conn: fix potential double free in le_scan_cleanup()
  2022-05-26 11:24 ` Paolo Abeni
@ 2022-05-26 20:11   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2022-05-26 20:11 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Jianglei Nie, marcel, johan.hedberg, davem, edumazet, kuba,
	linux-bluetooth, netdev, linux-kernel

Hi,

On Thu, May 26, 2022 at 4:24 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Thu, 2022-05-26 at 17:49 +0800, Jianglei Nie wrote:
> > When "c == conn" is true, hci_conn_cleanup() is called. The
> > hci_conn_cleanup() calls hci_dev_put() and hci_conn_put() in
> > its function implementation. hci_dev_put() and hci_conn_put()
> > will free the relevant resource if the reference count reaches
> > zero, which may lead to a double free when hci_dev_put() and
> > hci_conn_put() are called again.
> >
> > We should add a return to this function after hci_conn_cleanup()
> > is called.
> >
> > Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> > ---
> >  net/bluetooth/hci_conn.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index fe803bee419a..7b3e91eb9fa3 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -166,6 +166,7 @@ static void le_scan_cleanup(struct work_struct *work)
> >       if (c == conn) {
> >               hci_connect_le_scan_cleanup(conn);
> >               hci_conn_cleanup(conn);
> > +             return;
>
> This looks not correct. At very least you should release the
> hci_dev_lock.

Yep, it should probably use break instead of return.

> Cheers,
>
> Paolo
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-26 20:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26  9:49 [PATCH] Bluetooth: hci_conn: fix potential double free in le_scan_cleanup() Jianglei Nie
2022-05-26 11:24 ` Paolo Abeni
2022-05-26 20:11   ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.