Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4100-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240426072006.358802-1-iam@sung-woo.kim> <cff764c2-a3d1-4a12-9260-54122e7a1fef@moroto.mountain>
In-Reply-To: <cff764c2-a3d1-4a12-9260-54122e7a1fef@moroto.mountain>
From: Sungwoo Kim <iam@sung-woo.kim>
Date: Fri, 26 Apr 2024 05:35:01 -0400
Message-ID: <CAJNyHp+eCq1p_gTbxAJBKouAw-oQ=j5xwyjk5o-91pf_hGdrAg@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: L2CAP: Fix slab-use-after-free in l2cap_send_cmd
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: daveti@purdue.edu, Marcel Holtmann <marcel@holtmann.org>, 
	Johan Hedberg <johan.hedberg@gmail.com>, Luiz Augusto von Dentz <luiz.dentz@gmail.com>, 
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 26, 2024 at 5:16=E2=80=AFAM Dan Carpenter <dan.carpenter@linaro=
.org> wrote:
>
> On Fri, Apr 26, 2024 at 03:20:05AM -0400, Sungwoo Kim wrote:
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 84fc70862..a8f414ab8 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -3953,6 +3953,9 @@ static struct l2cap_chan *l2cap_connect(struct l2=
cap_conn *conn,
> >       if (!chan)
> >               goto response;
> >
> > +     l2cap_chan_hold(chan);
> > +     l2cap_chan_lock(chan);
> > +
> >       /* For certain devices (ex: HID mouse), support for authenticatio=
n,
> >        * pairing and bonding is optional. For such devices, inorder to =
avoid
> >        * the ACL alive for too long after L2CAP disconnection, reset th=
e ACL
> > @@ -4041,6 +4044,11 @@ static struct l2cap_chan *l2cap_connect(struct l=
2cap_conn *conn,
> >               chan->num_conf_req++;
> >       }
> >
> > +     if (chan) {
> > +             l2cap_chan_unlock(chan);
> > +             l2cap_chan_put(chan);
> > +     }
> > +
> >       return chan;
>         ^^^^^^^^^^^^
> This doesn't fix the bug because we're returning chan.
>
> As soon as you call l2cap_chan_put() then chan will be freed by in the
> other thread which is doing l2cap_conn_del() resulting in a use after
> free in the caller.

Thank you for pointing this out.
No caller uses the return value of l2cap_connect() if the kernel
versions >=3D v6.9.
So, l2cap_connect() can return void.

One caller uses the return value of l2cap_connect() in v4.19 <=3D the
kernel versions <=3D v6.8.
In this case, the caller should unlock and put a channel.

Question: Can different patches be applied for different versions like
the above?

Thanks,
Sungwoo Kim.