Bug 246964 - bhyve usb_mouse SIGSEGV on null pointer dereference
Summary: bhyve usb_mouse SIGSEGV on null pointer dereference
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: bhyve (show other bugs)
Version: 12.1-RELEASE
Hardware: amd64 Any
: --- Affects Some People
Assignee: freebsd-virtualization (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-03 18:56 UTC by Ali Abdallah
Modified: 2020-06-26 08:21 UTC (History)
1 user (show)

See Also:


Attachments
Fix null pointer dereference in usb_mouse (366 bytes, patch)
2020-06-03 18:56 UTC, Ali Abdallah
no flags Details | Diff
Fix null pointer dereference in usb_mouse (575 bytes, patch)
2020-06-03 19:06 UTC, Ali Abdallah
no flags Details | Diff
Fix null pointer dereferences in usb_mouse (1.01 KB, patch)
2020-06-04 06:57 UTC, Ali Abdallah
no flags Details | Diff
lsusb -v log (2.79 KB, text/plain)
2020-06-04 07:28 UTC, Peter Grehan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Abdallah 2020-06-03 18:56:02 UTC
Created attachment 215206 [details]
Fix null pointer dereference in usb_mouse

By chance when I was running 'supportconfig' on a openSUSE 15.1 guest under bhyve, it crashes with SIGSEGV. Debugging and checking the code revealed the problem. 

usb_mouse.c:538 eshort = data->blen > 0;

the data pointer above is accessed while being null. Attached is a trivial patch that fixes the issue for me.
Comment 1 Ali Abdallah 2020-06-03 19:06:44 UTC
Created attachment 215207 [details]
Fix null pointer dereference in usb_mouse

Actually there is also another null pointer dereference. Patch updated.
Comment 2 Peter Grehan freebsd_committer 2020-06-04 04:26:06 UTC
Thanks for the report. I was able to reproduce this so should be able to get your fix in.
Comment 3 Ali Abdallah 2020-06-04 06:57:08 UTC
Created attachment 215213 [details]
Fix null pointer dereferences in usb_mouse

Fix a couple of more dereferences in usb_mouse.

Please note that running 'scan-build80 make' in /usr/src/usr.sbin/bhyve reveals also other dereference of null pointers (no more in usb_mouse). I think those also can be potentially triggered in some way and should be fixed.
Comment 4 Peter Grehan freebsd_committer 2020-06-04 07:27:39 UTC
Sure, I'll give that a run.
Comment 5 Peter Grehan freebsd_committer 2020-06-04 07:28:42 UTC
Created attachment 215214 [details]
lsusb -v log
Comment 6 Peter Grehan freebsd_committer 2020-06-04 07:30:23 UTC
The command in supportconfig that resulted in the crash was "lsusb -vv", though an "lsusb -v" will also result in the core. Log attached from running that on an Ubuntu 20.04 live ISO.
Comment 7 Peter Grehan freebsd_committer 2020-06-08 11:06:18 UTC
The root cause of this issue was a bug in the XHCI TRB processing loop, which resulted in the back-end control transfer routine being called twice. While this was mostly benign, the GET_STATUS handler did not recognize this case and assumed a valid data pointer.

I have a patch for this that I'll put up for review. The NULL derefs will be fixed in a follow-up commit.
Comment 8 commit-hook freebsd_committer 2020-06-26 08:21:34 UTC
A commit references this bug:

Author: grehan
Date: Fri Jun 26 08:20:39 UTC 2020
New revision: 362644
URL: https://svnweb.freebsd.org/changeset/base/362644

Log:
  Prevent calling USB backends multiple times.

  The TRB processing loop could potentially call a back-end twice
  with the same status transaction. While this was generally benign,
  some code paths in the tablet backend weren't set up to handle
  this case, resulting in a NULL dereference.

  Fix by
   - returning a STALL error when an invalid request was seen in the backend
   - skipping a call to the backend if the number of packets in a status
     transaction was zero (this code fragment was taken from the Intel ACRN
     xhci backend)

  PR:	246964
  Reported by:  Ali Abdallah
  Discussed with: Leon Dang (author)
  Reviewed by: jhb (#bhyve), Leon Dang
  Approved by: jhb
  Obtained from:  Intel ACRN (partially)
  MFC after: 2 weeks
  Differential Revision:	https://reviews.freebsd.org/D25228

Changes:
  head/usr.sbin/bhyve/pci_xhci.c
  head/usr.sbin/bhyve/usb_mouse.c