trezor/cython-hidapi

MacOS panic when hid_write called with a zero length buffer

Closed this issue · 9 comments

I just opened this issue for libusb/hidapi and I wanted to share it with you folks. I do not think that cython-hidapi causes the panic, but I do suggest adding checks for zero-length buffers to I/O methods to avoid tickling the probable bug I'll put together a PR ;)

If libusb/hidapi handles that case and returns an error, could an additional check be skipped here?


P.S. I'm not familiar with Mac OS. Does Panicked task 0xffffffe199f04630: [...] pid 0: kernel_task mean that the panic happened in the kernel? If that's the case, it seems pretty serious...

It did indeed panic in the kernel which surprised me greatly. When I opened this issue, I wasn't sure how responsive the hidapi people would be. Turns out I shouldn't have worried. Once a check is in place for hidapi there's no reason for cython-hidapi to check for it too.

I've got a PR going right now to address the problem in hidapi. When that PR is accepted a hid_write with an empty buffer, bytes(0) in this particular case, will return a -1.

mcuee commented

A PR for HIDAPI will be very welcome. I am being hit by this myself.
Ref: libusb/hidapi#278

mcuee commented

PR here, it should be merged soon.
libusb/hidapi#279

My pull request addressing this bug and resultant kernel panic has been merged into libusb/hidapi. It should now be less difficult to induce a kernel panic going forward with cython-hidapi. 🤞

Looking at how cython-hidapi incorporates libusb/hidapi, some testing with the head needs to be done and then update the submodule hash to pickup these new changes.

mcuee commented

Updated the hidapi submodule to latest git and cython_hidapi seems to work fine.


(py39venv) C:\work\hid\cython-hidapi [master ≡ +0 ~1 -0 !]> git submodule update --remote --merge
remote: Enumerating objects: 16, done.
remote: Counting objects: 100% (16/16), done.
remote: Compressing objects: 100% (7/7), done.
remote: Total 16 (delta 9), reused 8 (delta 7), pack-reused 0
Unpacking objects: 100% (16/16), 4.61 KiB | 41.00 KiB/s, done.
From https://github.com/libusb/hidapi
   8741697..4d63a50  connection-callback -> origin/connection-callback
 * [new branch]      doc-upd             -> origin/doc-upd
   26d45bf..c6f4f74  get-descriptor      -> origin/get-descriptor

(py39venv) C:\work\hid\cython-hidapi [master ≡ +1 ~1 -0 !]> git diff
diff --git a/hidapi b/hidapi
index f6d0073..4d63a50 160000
--- a/hidapi
+++ b/hidapi
@@ -1 +1 @@
-Subproject commit f6d0073fcddbdda24549199445e844971d3c9cef
+Subproject commit 4d63a5085ac6e6a5be6f0f29a42a4e739da30881

(py39venv) C:\work\hid\cython-hidapi [master ≡ +0 ~1 -0 !]> python .\setup.py build
running build
running build_ext
skipping 'hid.c' Cython extension (up-to-date)
building 'hid' extension
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\bin\HostX86\x64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -Ihidapi\hidapi -IC:\work\python\py39venv\include -IC:\Python39\include -IC:\Python39\include -IC:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\ATLMFC\include -IC:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\include -IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\ucrt -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\shared -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\um -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\winrt -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\cppwinrt /Tchid.c /Fobuild\temp.win-amd64-3.9\Release\hid.obj
hid.c
hid.c(1476): warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data
hid.c(2098): warning C4244: '=': conversion from 'Py_ssize_t' to 'int', possible loss of data
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\bin\HostX86\x64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -Ihidapi\hidapi -IC:\work\python\py39venv\include -IC:\Python39\include -IC:\Python39\include -IC:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\ATLMFC\include -IC:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\include -IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\ucrt -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\shared -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\um -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\winrt -IC:\Program Files (x86)\Windows Kits\10\include\10.0.20348.0\cppwinrt /Tchidapi\windows\hid.c /Fobuild\temp.win-amd64-3.9\Release\hidapi\windows\hid.obj
hid.c
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\bin\HostX86\x64\link.exe /nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO /LIBPATH:C:\work\python\py39venv\libs /LIBPATH:C:\Python39\libs /LIBPATH:C:\Python39 /LIBPATH:C:\work\python\py39venv\PCbuild\amd64 /LIBPATH:C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\ATLMFC\lib\x64 /LIBPATH:C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30037\lib\x64 /LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64 /LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.20348.0\ucrt\x64 /LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.20348.0\um\x64 setupapi.lib /EXPORT:PyInit_hid build\temp.win-amd64-3.9\Release\hid.obj build\temp.win-amd64-3.9\Release\hidapi\windows\hid.obj /OUT:build\lib.win-amd64-3.9\hid.cp39-win_amd64.pyd /IMPLIB:build\temp.win-amd64-3.9\Release\hid.cp39-win_amd64.lib
   Creating library build\temp.win-amd64-3.9\Release\hid.cp39-win_amd64.lib and object build\temp.win-amd64-3.9\Release\hid.cp39-win_amd64.exp
Generating code
Finished generating code
(py39venv) C:\work\hid\cython-hidapi [master ≡ +0 ~1 -0 !]> python .\setup.py install
running install
running bdist_egg
running egg_info
writing hidapi.egg-info\PKG-INFO
writing dependency_links to hidapi.egg-info\dependency_links.txt
writing requirements to hidapi.egg-info\requires.txt
writing top-level names to hidapi.egg-info\top_level.txt
reading manifest file 'hidapi.egg-info\SOURCES.txt'
reading manifest template 'MANIFEST.in'
adding license file 'LICENSE-bsd.txt'
adding license file 'LICENSE-gpl3.txt'
adding license file 'LICENSE-orig.txt'
adding license file 'LICENSE.txt'
writing manifest file 'hidapi.egg-info\SOURCES.txt'
installing library code to build\bdist.win-amd64\egg
running install_lib
running build_ext
skipping 'hid.c' Cython extension (up-to-date)
creating build\bdist.win-amd64\egg
copying build\lib.win-amd64-3.9\hid.cp39-win_amd64.pyd -> build\bdist.win-amd64\egg
creating stub loader for hid.cp39-win_amd64.pyd
byte-compiling build\bdist.win-amd64\egg\hid.py to hid.cpython-39.pyc
creating build\bdist.win-amd64\egg\EGG-INFO
copying hidapi.egg-info\PKG-INFO -> build\bdist.win-amd64\egg\EGG-INFO
copying hidapi.egg-info\SOURCES.txt -> build\bdist.win-amd64\egg\EGG-INFO
copying hidapi.egg-info\dependency_links.txt -> build\bdist.win-amd64\egg\EGG-INFO
copying hidapi.egg-info\requires.txt -> build\bdist.win-amd64\egg\EGG-INFO
copying hidapi.egg-info\top_level.txt -> build\bdist.win-amd64\egg\EGG-INFO
writing build\bdist.win-amd64\egg\EGG-INFO\native_libs.txt
zip_safe flag not set; analyzing archive contents...
__pycache__.hid.cpython-39: module references __file__
creating 'dist\hidapi-0.10.1-py3.9-win-amd64.egg' and adding 'build\bdist.win-amd64\egg' to it
removing 'build\bdist.win-amd64\egg' (and everything under it)
Processing hidapi-0.10.1-py3.9-win-amd64.egg
removing 'c:\work\python\py39venv\lib\site-packages\hidapi-0.10.1-py3.9-win-amd64.egg' (and everything under it)
creating c:\work\python\py39venv\lib\site-packages\hidapi-0.10.1-py3.9-win-amd64.egg
Extracting hidapi-0.10.1-py3.9-win-amd64.egg to c:\work\python\py39venv\lib\site-packages
hidapi 0.10.1 is already the active version in easy-install.pth

Installed c:\work\python\py39venv\lib\site-packages\hidapi-0.10.1-py3.9-win-amd64.egg
Processing dependencies for hidapi==0.10.1
Searching for setuptools==57.4.0
Best match: setuptools 57.4.0
Adding setuptools 57.4.0 to easy-install.pth file

Using c:\work\python\py39venv\lib\site-packages
Finished processing dependencies for hidapi==0.10.1

(py39venv) C:\work\libusb> python
Python 3.9.6 (tags/v3.9.6:db3ff76, Jun 28 2021, 15:26:21) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import hid
>>> hid.enumerate(0x0925,0x7001)
[{'path': b'\\\\?\\hid#vid_0925&pid_7001#6&461d29f&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}', 'vendor_id': 2341, 'product_id': 28673, 'serial_number': '', 'release_number': 1, 'manufacturer_string': 'Microchip Technology Inc.', 'product_string': 'Generic HID', 'usage_page': 65440, 'usage': 1, 'interface_number': -1}]
>>> d = hid.device()
>>> d.open_path(b'\\\\?\\hid#vid_0925&pid_7001#6&461d29f&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}')
>>> d.write([0,0x40,0x41]
... )
3
>>> h=d.read(3)
>>> print(h)
[64, 65]
>>> d.set_nonblocking(1)
0
>>> d.write([0,0x40,0x41])
3
>>> h=d.read(3)
>>> print(h)
[64, 65]



mcuee commented

There is a very important fix in HIDAPI recently for macOS: libusb/hidapi#322 which fixes libusb/hidapi#236.
Without this fix, hidapi is almost not usable for me as I have external USB hubs.

Without the fix: try.py output will have something like the following (empty path).

interface_number : 2
manufacturer_string : Logitech
path : b''
product_id : 50475
product_string : USB Receiver
release_number : 9232
serial_number : 
usage : 1
usage_page : 65280
vendor_id : 1133

With the fix, the path is now not empty.

interface_number : 2
manufacturer_string : Logitech
path : b'DevSrvsID:4294969770'
product_id : 50475
product_string : USB Receiver
release_number : 9232
serial_number : 
usage : 1
usage_page : 65280
vendor_id : 1133```

Thanks for the info. Closing this because the issue is now resolved in the upstream. Hopefully they will cut a new release soon.