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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 2 Aug 2021 13:38:58 +0200
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:     Hillf Danton <hdanton@...a.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        Johan Hedberg <johan.hedberg@...il.com>,
        BlueZ <linux-bluetooth@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Luiz Augusto von Dentz <luiz.von.dentz@...el.com>,
        netdev@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        syzbot <syzbot+47c6d0efbb7fe2f7a5b8@...kaller.appspotmail.com>
Subject: Re: [syzbot] general protection fault in hci_release_dev

Hi Tetsuo,

>> To fix what was addressed in e305509e678b3a4a, defer putting hdev until
>> sock is released with sock locked.
>> 
>> Now only for thoughts.
> 
> Thanks for your analysis.
> 
> hci_alloc_dev() is called from hci_uart_register_dev() from  hci_uart_set_proto()
> from hci_uart_tty_ioctl(HCIUARTSETPROTO) via ld->ops->ioctl() from tty_ioctl(),
> and bt_host_release() is called from device_release() from kobject_put() from
> hci_uart_tty_close() from tty_ldisc_kill() from tty_ldisc_release() from
> tty_release_struct() from tty_release() from __fput().
> 
> The problem is that bt_host_release() is expecting that hci_register_dev()
> was called if "struct hci_dev" was allocated by hci_alloc_dev(). In other
> words, hci_register_dev() might not be called before bt_host_release().
> 
> Then, the fix I think is not to call hci_release_dev() when hci_unregister_dev()
> was not called. That is,
> 
> static void bt_host_release(struct device *dev)
> {
>        struct hci_dev *hdev = to_hci_dev(dev);
> +
> +       if (hci_dev_test_flag(hdev, HCI_UNREGISTER))
> +               hci_release_dev(hdev);
>        kfree(hdev);
>        module_put(THIS_MODULE);
> }
> 
> and remove kfree(hdev) from hci_release_dev(), for HCI_UNREGISTER flag is
> set if hci_unregister_dev() was called before bt_host_release() is called.

actually I am wondering if we should just remove the HCI LDISC support. All the tests
are focusing around the fact that you can create a line discipline as unprivileged
user.

To be honest the HCI LDISC support is not in use anymore for anything deployed after
we got around to establish TTY serdev support.

I am worried that we are trying hard to fix something in the Bluetooth core that
is actually a bug in the hci_uart driver and should be fixed solely there. Or that
driver needs to be deprecated. Are other drivers and their lifetime rules also
exhibiting these issues?

Regards

Marcel

Powered by blists - more mailing lists