[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230705093636.GL5866@linux-l9pv.suse>
Date: Wed, 5 Jul 2023 17:36:36 +0800
From: joeyli <jlee@...e.com>
To: Weiteng Chen <wchen130@....edu>, Yu Hao <yhao016@....edu>
Cc: linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: How to reproduce the BUG general protection fault in
hci_uart_tty_ioctl?
On Wed, Jul 05, 2023 at 04:41:48PM +0800, joeyli wrote:
> Hi Weiteng Chen, Yu Hao,
>
> On Mon, Jul 03, 2023 at 09:07:38PM -0700, Weiteng Chen wrote:
> > Hi Joey,
> >
> > Sorry for my late response.
> >
> > https://elixir.bootlin.com/linux/v6.3-rc7/source/drivers/bluetooth/hci_ldisc.c#L764
> >
> > switch (cmd) {
> > case HCIUARTSETPROTO:
> > if (!test_and_set_bit(HCI_UART_PROTO_SET, &hu->flags)) {
> > printk(“test_and_set_bit…”) // insert a prink to make the race easy to trigger
> > err = hci_uart_set_proto(hu, arg);
> > if (err)
> > clear_bit(HCI_UART_PROTO_SET, &hu->flags);
> > } else
> > err = -EBUSY;
> > break;
> >
> > case HCIUARTGETPROTO:
> > if (test_bit(HCI_UART_PROTO_SET, &hu->flags))
> > err = hu->proto->id; ←- null pointer deference
> > else
> > err = -EUNATCH;
> > break;
> >
> > This is a race condition between HCIUARTSETPROTO and HCIUARTGETPROTO. HCI_UART_PROTO_SET is set before hu->proto is set and thus it may dereference a null pointer.
> >
> > To easily trigger this bug, I inserted a prink in the source code so that the C producer can easily trigger the bug. Please let me know if you have any questions.
> >
>
> Thanks! I can reproduce the issue now.
>
> Weiteng, Yu Hao, do you have plan for sending patch to fix this problem?
>
> Joey Lee
Looks that check HCI_UART_PROTO_READY is enough to avoid problem:
--- linux.orig/drivers/bluetooth/hci_ldisc.c
+++ linux/drivers/bluetooth/hci_ldisc.c
@@ -771,7 +771,7 @@ static int hci_uart_tty_ioctl(struct tty
break;
case HCIUARTGETPROTO:
- if (test_bit(HCI_UART_PROTO_SET, &hu->flags))
+ if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
err = hu->proto->id;
else
err = -EUNATCH;
If you do not have plan to send patch, then I will send the above change.
Thanks
Joey Lee
>
>
> > Best,
> > Weiteng Chen
> >
> > > On Jul 3, 2023, at 8:01 PM, joeyli <jlee@...e.com> wrote:
> > >
> > > Hi,
> > >
> > > On Wed, Jun 28, 2023 at 06:57:47PM -0700, Yu Hao wrote:
> > >> Hi Weiteng,
> > >>
> > >> Could you give more info about the bug, e.g., kernel configuration,
> > >> qemu arguments.
> > >>
> > >
> > > Base on kernel code, looks that the HCIUARTSETPROTO and HCIUARTGETPROTO
> > > blocks in hci_uart_tty_ioctl() should use hci_uart->proto_lock.
> > >
> > > I have run the C reproducer a couple of days in qemu, but it did not
> > > reproduce issue until now.
> > >
> > > Does anyone know how to reproduce this issue easily?
> > >
> > > Thanks
> > > Joey Lee
> > >>
> > >> On Wed, Jun 28, 2023 at 8:02 AM joeyli <jlee@...e.com> wrote:
> > >>>
> > >>> Hi Yu Hao,
> > >>>
> > >>> I am looking at your "BUG: general protection fault in hci_uart_tty_ioctl":
> > >>>
> > >>> https://lore.kernel.org/all/CA+UBctC3p49aTgzbVgkSZ2+TQcqq4fPDO7yZitFT5uBPDeCO2g@mail.gmail.com/
> > >>>
> > >>> I am trying the C reproducer in your URL, but it is not success yet:
> > >>> https://gist.github.com/ZHYfeng/a3e3ff2bdfea5ed5de5475f0b54d55cb
> > >>>
> > >>> I am using v6.2 mainline kernel to run the C reproducer.
> > >>>
> > >>> Could you please provide suggestions for how to reproduce this issue?
> > >>> And what is your qemu environment for reproducing issue?
> > >>>
> > >>> Thanks a lot!
> > >>> Joey Lee
Powered by blists - more mailing lists