linux-kernel - Re: [PATCH 4.9 02/31] Bluetooth: hci_ldisc: Postpone HCI_UART_PROTO_READY bit set in hci_uart_set

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]

Date:   Fri, 15 Nov 2019 11:10:29 -0500
From:   Ralph Siemsen <ralph.siemsen@...aro.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        syzbot+899a33dc0fa0dbaf06a6@...kaller.appspotmail.com,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Jeremy Cline <jcline@...hat.com>,
        Marcel Holtmann <marcel@...tmann.org>
Subject: Re: [PATCH 4.9 02/31] Bluetooth: hci_ldisc: Postpone
 HCI_UART_PROTO_READY bit set in hci_uart_set_proto()

Hi Greg,

On Fri, Nov 15, 2019 at 02:20:31PM +0800, Greg Kroah-Hartman wrote:
>From: Kefeng Wang <wangkefeng.wang@...wei.com>
>
>commit 56897b217a1d0a91c9920cb418d6b3fe922f590a upstream.
>
>task A:                                task B:
>hci_uart_set_proto                     flush_to_ldisc
> - p->open(hu) -> h5_open  //alloc h5  - receive_buf
> - set_bit HCI_UART_PROTO_READY         - tty_port_default_receive_buf
> - hci_uart_register_dev                 - tty_ldisc_receive_buf
>                                          - hci_uart_tty_receive
>				           - test_bit HCI_UART_PROTO_READY
>				            - h5_recv
> - clear_bit HCI_UART_PROTO_READY             while() {
> - p->open(hu) -> h5_close //free h5
>				              - h5_rx_3wire_hdr
>				               - h5_reset()  //use-after-free
>                                              }
>
>It could use ioctl to set hci uart proto, but there is
>a use-after-free issue when hci_uart_register_dev() fail in
>hci_uart_set_proto(), see stack above, fix this by setting
>HCI_UART_PROTO_READY bit only when hci_uart_register_dev()
>return success.
>
>Reported-by: syzbot+899a33dc0fa0dbaf06a6@...kaller.appspotmail.com
>Signed-off-by: Kefeng Wang <wangkefeng.wang@...wei.com>
>Reviewed-by: Jeremy Cline <jcline@...hat.com>
>Signed-off-by: Marcel Holtmann <marcel@...tmann.org>
>Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

I was just about to ask why this had not been merged into 4.9. Spent a 
while searching archives for any discussion to explain its absence, but 
couldn't find anything. Also watched your kernel-recipes talk...

BTW, this also seems to be missing from 4.4 branch, although it was 
merged for 3.16 (per https://lore.kernel.org/stable/?q=Postpone+HCI).

I gather that the usual rule is that a fix must be in newer versions 
before it can go into older ones. Or at least, some patches were 
rejected on that basis. If this is in fact the policy, perhaps it could 
be added to stable-kernel-rules.rst ?

-Ralph

>---
> drivers/bluetooth/hci_ldisc.c |    3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>--- a/drivers/bluetooth/hci_ldisc.c
>+++ b/drivers/bluetooth/hci_ldisc.c
>@@ -653,15 +653,14 @@ static int hci_uart_set_proto(struct hci
> 		return err;
>
> 	hu->proto = p;
>-	set_bit(HCI_UART_PROTO_READY, &hu->flags);
>
> 	err = hci_uart_register_dev(hu);
> 	if (err) {
>-		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
> 		p->close(hu);
> 		return err;
> 	}
>
>+	set_bit(HCI_UART_PROTO_READY, &hu->flags);
> 	return 0;
> }
>
>
>