Re: [PATCH v2] Bluetooth: vhci, fix open_timeout vs. hdev race

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jiri,

> Both vhci_get_user and vhci_release race with open_timeout work. They
> both contain cancel_delayed_work_sync, but do not test whether the
> work actually created hdev or not. Since the work can be in progress
> and _sync will wait for finishing it, we can have data->hdev allocated
> when cancel_delayed_work_sync returns. But the call sites do 'if
> (data->hdev)' *before* cancel_delayed_work_sync.
> 
> As a result:
> * vhci_get_user allocates a second hdev and puts it into
>  data->hdev. The former is leaked.
> * vhci_release does not release data->hdev properly as it thinks there
>  is none.
> 
> Fix both cases by moving the actual test *after* the call to
> cancel_delayed_work_sync.
> 
> This can be hit by this program:
> 	#include <err.h>
> 	#include <fcntl.h>
> 	#include <stdio.h>
> 	#include <stdlib.h>
> 	#include <time.h>
> 	#include <unistd.h>
> 
> 	#include <sys/stat.h>
> 	#include <sys/types.h>
> 
> 	int main(int argc, char **argv)
> 	{
> 		int fd;
> 
> 		srand(time(NULL));
> 
> 		while (1) {
> 			const int delta = (rand() % 200 - 100) * 100;
> 
> 			fd = open("/dev/vhci", O_RDWR);
> 			if (fd < 0)
> 				err(1, "open");
> 
> 			usleep(1000000 + delta);
> 
> 			close(fd);
> 		}
> 
> 		return 0;
> 	}
> 
> And the result is:
> BUG: KASAN: use-after-free in skb_queue_tail+0x13e/0x150 at addr ffff88006b0c1228
> Read of size 8 by task kworker/u13:1/32068
> =============================================================================
> BUG kmalloc-192 (Tainted: G            E     ): kasan: bad access detected
> -----------------------------------------------------------------------------
> 
> Disabling lock debugging due to kernel taint
> INFO: Allocated in vhci_open+0x50/0x330 [hci_vhci] age=260 cpu=3 pid=32040
> ...
> 	kmem_cache_alloc_trace+0x150/0x190
> 	vhci_open+0x50/0x330 [hci_vhci]
> 	misc_open+0x35b/0x4e0
> 	chrdev_open+0x23b/0x510
> ...
> INFO: Freed in vhci_release+0xa4/0xd0 [hci_vhci] age=9 cpu=2 pid=32040
> ...
> 	__slab_free+0x204/0x310
> 	vhci_release+0xa4/0xd0 [hci_vhci]
> ...
> INFO: Slab 0xffffea0001ac3000 objects=16 used=13 fp=0xffff88006b0c1e00 flags=0x5fffff80004080
> INFO: Object 0xffff88006b0c1200 @offset=4608 fp=0xffff88006b0c0600
> Bytes b4 ffff88006b0c11f0: 09 df 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
> Object ffff88006b0c1200: 00 06 0c 6b 00 88 ff ff 00 00 00 00 00 00 00 00  ...k............
> Object ffff88006b0c1210: 10 12 0c 6b 00 88 ff ff 10 12 0c 6b 00 88 ff ff  ...k.......k....
> Object ffff88006b0c1220: c0 46 c2 6b 00 88 ff ff c0 46 c2 6b 00 88 ff ff  .F.k.....F.k....
> Object ffff88006b0c1230: 01 00 00 00 01 00 00 00 e0 ff ff ff 0f 00 00 00  ................
> Object ffff88006b0c1240: 40 12 0c 6b 00 88 ff ff 40 12 0c 6b 00 88 ff ff  @..k....@..k....
> Object ffff88006b0c1250: 50 0d 6e a0 ff ff ff ff 00 02 00 00 00 00 ad de  P.n.............
> Object ffff88006b0c1260: 00 00 00 00 00 00 00 00 ab 62 02 00 01 00 00 00  .........b......
> Object ffff88006b0c1270: 90 b9 19 81 ff ff ff ff 38 12 0c 6b 00 88 ff ff  ........8..k....
> Object ffff88006b0c1280: 03 00 20 00 ff ff ff ff ff ff ff ff 00 00 00 00  .. .............
> Object ffff88006b0c1290: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> Object ffff88006b0c12a0: 00 00 00 00 00 00 00 00 00 80 cd 3d 00 88 ff ff  ...........=....
> Object ffff88006b0c12b0: 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00  . ..............
> Redzone ffff88006b0c12c0: bb bb bb bb bb bb bb bb                          ........
> Padding ffff88006b0c13f8: 00 00 00 00 00 00 00 00                          ........
> CPU: 3 PID: 32068 Comm: kworker/u13:1 Tainted: G    B       E      4.4.6-0-default #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20151112_172657-sheep25 04/01/2014
> Workqueue: hci0 hci_cmd_work [bluetooth]
> 00000000ffffffff ffffffff81926cfa ffff88006be37c68 ffff88006bc27180
> ffff88006b0c1200 ffff88006b0c1234 ffffffff81577993 ffffffff82489320
> ffff88006bc24240 0000000000000046 ffff88006a100000 000000026e51eb80
> Call Trace:
> ...
> [<ffffffff81ec8ebe>] ? skb_queue_tail+0x13e/0x150
> [<ffffffffa06e027c>] ? vhci_send_frame+0xac/0x100 [hci_vhci]
> [<ffffffffa0c61268>] ? hci_send_frame+0x188/0x320 [bluetooth]
> [<ffffffffa0c61515>] ? hci_cmd_work+0x115/0x310 [bluetooth]
> [<ffffffff811a1375>] ? process_one_work+0x815/0x1340
> [<ffffffff811a1f85>] ? worker_thread+0xe5/0x11f0
> [<ffffffff811a1ea0>] ? process_one_work+0x1340/0x1340
> [<ffffffff811b3c68>] ? kthread+0x1c8/0x230
> ...
> Memory state around the buggy address:
> ffff88006b0c1100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88006b0c1180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88006b0c1200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                                  ^
> ffff88006b0c1280: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ffff88006b0c1300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> 
> Fixes: 23424c0d31 (Bluetooth: Add support creating virtual AMP controllers)
> Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
> Cc: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx>
> Cc: Johan Hedberg <johan.hedberg@xxxxxxxxx>
> Cc: <linux-bluetooth@xxxxxxxxxxxxxxx>
> Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Cc: stable 3.13+ <stable@xxxxxxxxxxxxxxx>
> ---
> [v2] describe the symptoms and cleaner program
> 
> drivers/bluetooth/hci_vhci.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]