Skip to content

Commit

Permalink
Make hidapi_exit() safe by turning it into a no-op
Browse files Browse the repository at this point in the history
Calling hid_exit() can lead to memory bugs if care is not taken.  This
has been observed as segmentation faults with the LibUSB backend, where
hid_exit() calls libusb_exit(), which in turn has a well documented
contract:

> Deinitialize libusb. Should be called after closing all open devices
> and before your application terminates.

While the Linux HIDRAW backend appears to be more tolerant, there may be
other backends that behave like LibUSB, or even have additional
prerequisites for calling hid_exit().

On its part, HIDAPI is not clear about the requirements of hid_exit():

> This function frees all of the static data associated with HIDAPI. It
> should be called at the end of execution to avoid memory leaks.

Because of these reasons, there is no alternative but to threat
hid_exit() as a very unsafe function.

As such, any wrapper we provide for it – in practice, hidapi_exit() –
would also have to be made safe.  Unfortunately, doing that is
non-trivial, and possibly quite expensive.

Instead, turn hidapi_exit() into a no-op for now.  The HIDAPI library
will still be cleanly finalized before the program terminates, through
another – automatic – mechanism.

Related: trezor#128 ("Fatal Python error: Segmentation fault on 0.11.0.post2")
  • Loading branch information
jonasmalacofilho committed Nov 2, 2021
1 parent 621d285 commit d1eb068
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions hid.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ def enumerate(int vendor_id=0, int product_id=0):
return res

def hidapi_exit():
"""Callback for when the script exits.
"""No-op.
This prevents memory leaks in the hidapi C library.
Note that the counterpart, hid_init(), is not called explicitly. It will
be called internally when first needed.
Turned into a no-op to prevent memory safety bugs (see issue #128).
HIDAPI will be finalized automatically, when appropriate.
"""
hid_exit()

cdef class device:
"""Device class.
Expand Down Expand Up @@ -396,4 +395,4 @@ cdef class device:

# Finalize the HIDAPI library *only* once there are no more references to this
# module, and it is being garbage collected.
weakref.finalize(sys.modules[__name__], hidapi_exit)
weakref.finalize(sys.modules[__name__], hid_exit)

0 comments on commit d1eb068

Please sign in to comment.