linux-kernel - Re: [PATCH] Bluetooth/l2cap: Fix uaf in l2cap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANp29Y7TBdGuThfBX1=K7jNj6_+0XqL0A3_=sF1xMMpMWNfoYg@mail.gmail.com>
Date: Mon, 23 Sep 2024 17:38:03 +0200
From: Aleksandr Nogikh <nogikh@...gle.com>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Hillf Danton <hdanton@...a.com>, Edward Adam Davis <eadavis@...com>, 
	syzbot+c12e2f941af1feb5632c@...kaller.appspotmail.com, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] Bluetooth/l2cap: Fix uaf in l2cap_connect

Hi Luiz,

On Mon, Sep 23, 2024 at 5:20 PM Luiz Augusto von Dentz
<luiz.dentz@...il.com> wrote:
>
> Hi Aleksandr,
>
> On Mon, Sep 23, 2024 at 10:37 AM Aleksandr Nogikh <nogikh@...gle.com> wrote:
> >
> > On Mon, Sep 23, 2024 at 4:33 PM Luiz Augusto von Dentz
> > <luiz.dentz@...il.com> wrote:
> > >
> > > Hi Hillf,
> > >
> > > On Sat, Sep 21, 2024 at 6:56 AM Hillf Danton <hdanton@...a.com> wrote:
> > > >
> > > > On Tue, Sep 10, 2024 at 4:56 PM Luiz Augusto von Dentz <luiz.dentz@...il.com> wrote:
> > > > >
> > > > > Maybe something like the following would be a better approach:
> > > > >
> > > > > https://gist.github.com/Vudentz/121a15fa4391b2b1f6c7e8d420a6846e
> > > >
> > > > If your idea is not bad, boy, feel free to win Tested-by from syzbot with it.
> > >
> > > Is there a way to quickly check a patch with syzbot?
> >
> > You can send a `#syz test` command in a reply to syzbot and attach the
> > patch-to-test to the email message.
> >
> > See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches
>
> Thanks, lets see if this works:
>
> #syz test
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index d6976db02c06..b2f8f9c5b610 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3782,6 +3782,8 @@ static void hci_acldata_packet(struct hci_dev
> *hdev, struct sk_buff *skb)
>
>         hci_dev_lock(hdev);
>         conn = hci_conn_hash_lookup_handle(hdev, handle);
> +       if (conn && hci_dev_test_flag(hdev, HCI_MGMT))
> +               mgmt_device_connected(hdev, conn, NULL, 0);
>         hci_dev_unlock(hdev);
>

^^ Patch parsing will fail here because it expects to see the git diff
output as is -- i.e. if some line only consisted of a single
whitespace (= it was an empty line and it did not change), it must
remain so. Sometimes these whitespaces get lost during copy-pasting
and it confuses syzbot.

>         if (conn) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 1c82dcdf6e8f..b87c0f1dab9e 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3706,7 +3706,7 @@ static void hci_remote_features_evt(struct
> hci_dev *hdev, void *data,
>                 goto unlock;
>         }
>
> -       if (!ev->status && !test_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) {
> +       if (!ev->status) {
>                 struct hci_cp_remote_name_req cp;
>                 memset(&cp, 0, sizeof(cp));
>                 bacpy(&cp.bdaddr, &conn->dst);
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9988ba382b68..6544c1ed7143 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4066,17 +4066,9 @@ static void l2cap_connect(struct l2cap_conn
> *conn, struct l2cap_cmd_hdr *cmd,
>  static int l2cap_connect_req(struct l2cap_conn *conn,
>                              struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data)
>  {
> -       struct hci_dev *hdev = conn->hcon->hdev;
> -       struct hci_conn *hcon = conn->hcon;
> -
>         if (cmd_len < sizeof(struct l2cap_conn_req))
>                 return -EPROTO;
>
> -       hci_dev_lock(hdev);
> -       if (hci_dev_test_flag(hdev, HCI_MGMT))
> -               mgmt_device_connected(hdev, hcon, NULL, 0);
> -       hci_dev_unlock(hdev);
> -
>         l2cap_connect(conn, cmd, data, L2CAP_CONN_RSP);
>         return 0;
>  }

-- 
Aleksandr