Skip to content

Commit

Permalink
firewire: nosy: Fix a use-after-free bug in nosy_ioctl()
Browse files Browse the repository at this point in the history
For each device, the nosy driver allocates a pcilynx structure.
A use-after-free might happen in the following scenario:

 1. Open nosy device for the first time and call ioctl with command
    NOSY_IOC_START, then a new client A will be malloced and added to
    doubly linked list.
 2. Open nosy device for the second time and call ioctl with command
    NOSY_IOC_START, then a new client B will be malloced and added to
    doubly linked list.
 3. Call ioctl with command NOSY_IOC_START for client A, then client A
    will be readded to the doubly linked list. Now the doubly linked
    list is messed up.
 4. Close the first nosy device and nosy_release will be called. In
    nosy_release, client A will be unlinked and freed.
 5. Close the second nosy device, and client A will be referenced,
    resulting in UAF.

The root cause of this bug is that the element in the doubly linked list
is reentered into the list.

Fix this bug by adding a check before inserting a client.  If a client
is already in the linked list, don't insert it.

The following KASAN report reveals it:

   BUG: KASAN: use-after-free in nosy_release+0x1ea/0x210
   Write of size 8 at addr ffff888102ad7360 by task poc
   CPU: 3 PID: 337 Comm: poc Not tainted 5.12.0-rc5+ torvalds#6
   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
   Call Trace:
     nosy_release+0x1ea/0x210
     __fput+0x1e2/0x840
     task_work_run+0xe8/0x180
     exit_to_user_mode_prepare+0x114/0x120
     syscall_exit_to_user_mode+0x1d/0x40
     entry_SYSCALL_64_after_hwframe+0x44/0xae

   Allocated by task 337:
     nosy_open+0x154/0x4d0
     misc_open+0x2ec/0x410
     chrdev_open+0x20d/0x5a0
     do_dentry_open+0x40f/0xe80
     path_openat+0x1cf9/0x37b0
     do_filp_open+0x16d/0x390
     do_sys_openat2+0x11d/0x360
     __x64_sys_open+0xfd/0x1a0
     do_syscall_64+0x33/0x40
     entry_SYSCALL_64_after_hwframe+0x44/0xae

   Freed by task 337:
     kfree+0x8f/0x210
     nosy_release+0x158/0x210
     __fput+0x1e2/0x840
     task_work_run+0xe8/0x180
     exit_to_user_mode_prepare+0x114/0x120
     syscall_exit_to_user_mode+0x1d/0x40
     entry_SYSCALL_64_after_hwframe+0x44/0xae

   The buggy address belongs to the object at ffff888102ad7300 which belongs to the cache kmalloc-128 of size 128
   The buggy address is located 96 bytes inside of 128-byte region [ffff888102ad7300, ffff888102ad7380)

[ Modified to use 'list_empty()' inside proper lock  - Linus ]

Link: https://lore.kernel.org/lkml/[email protected]/
Reported-and-tested-by: 马哲宇 (Zheyu Ma) <[email protected]>
Signed-off-by: Zheyu Ma <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Stefan Richter <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
ZheyuMa authored and torvalds committed Apr 4, 2021
1 parent 2023a53 commit 829933e
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions drivers/firewire/nosy.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ nosy_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct client *client = file->private_data;
spinlock_t *client_list_lock = &client->lynx->client_list_lock;
struct nosy_stats stats;
int ret;

switch (cmd) {
case NOSY_IOC_GET_STATS:
Expand All @@ -360,11 +361,15 @@ nosy_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return 0;

case NOSY_IOC_START:
ret = -EBUSY;
spin_lock_irq(client_list_lock);
list_add_tail(&client->link, &client->lynx->client_list);
if (list_empty(&client->link)) {
list_add_tail(&client->link, &client->lynx->client_list);
ret = 0;
}
spin_unlock_irq(client_list_lock);

return 0;
return ret;

case NOSY_IOC_STOP:
spin_lock_irq(client_list_lock);
Expand Down

0 comments on commit 829933e

Please sign in to comment.