linux-kernel - Re: KASAN: use-after-free Write in hiddev

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]

Message-ID: <20200114152414.GC3719@kadam>
Date:   Tue, 14 Jan 2020 18:24:14 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     syzbot <syzbot+784ccb935f9900cc7c9e@...kaller.appspotmail.com>
Cc:     andreyknvl@...gle.com, benjamin.tissoires@...hat.com,
        jikos@...nel.org, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        syzkaller-bugs@...glegroups.com
Subject: Re: KASAN: use-after-free Write in hiddev_disconnect

I'm trying to take a stab at diagnosing these bugs.  (But I'm seldom
smart enough to actually fix anything).  These hiddev_disconnect() bugs
are a race condition:

devel/drivers/hid/usbhid/hiddev.c
   924  void hiddev_disconnect(struct hid_device *hid)
   925  {
   926          struct hiddev *hiddev = hid->hiddev;
   927          struct usbhid_device *usbhid = hid->driver_data;
   928  
   929          usb_deregister_dev(usbhid->intf, &hiddev_class);
   930  
   931          mutex_lock(&hiddev->existancelock);
   932          hiddev->exist = 0;
                ^^^^^^^^^^^^^^^^^^
We set "exist = 0;"

   933  
   934          if (hiddev->open) {
   935                  mutex_unlock(&hiddev->existancelock);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Then we drop the lock.

   936                  hid_hw_close(hiddev->hid);
   937                  wake_up_interruptible(&hiddev->wait);
                                               ^^^^^^
The other thread frees hiddev and it crashes here.

   938          } else {
   939                  mutex_unlock(&hiddev->existancelock);
   940                  kfree(hiddev);
   941          }
   942  }

The other thread is doing:

drivers/hid/usbhid/hiddev.c
   216  static int hiddev_release(struct inode * inode, struct file * file)
   217  {
   218          struct hiddev_list *list = file->private_data;
   219          unsigned long flags;
   220  
   221          spin_lock_irqsave(&list->hiddev->list_lock, flags);
   222          list_del(&list->node);
   223          spin_unlock_irqrestore(&list->hiddev->list_lock, flags);
   224  
   225          mutex_lock(&list->hiddev->existancelock);
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Takes the lock.

   226          if (!--list->hiddev->open) {
                     ^^^^^^^^^^^^^^^^^^^^
Decrements open.

   227                  if (list->hiddev->exist) {
                            ^^^^^^^^^^^^^^^^^^^
This is false.

   228                          hid_hw_close(list->hiddev->hid);
   229                          hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
   230                  } else {
   231                          mutex_unlock(&list->hiddev->existancelock);
   232                          kfree(list->hiddev);
                                      ^^^^^^^^^^^^
Freed here.

   233                          vfree(list);
   234                          return 0;
   235                  }
   236          }
   237  
   238          mutex_unlock(&list->hiddev->existancelock);
   239          vfree(list);
   240  
   241          return 0;
   242  }

I'm not sure what the fix should be though.

regards,
dan carpenter