9576 hid_attach is missing one mutex_exit in error path

Review Request #1097 — Created June 2, 2018 and submitted

wiedi
illumos-gate
general

Issue: https://www.illumos.org/issues/9576

In hid_attach() there is one error path leaving the function without releasing a mutex.
Since I don't think it is possible to hit this error path with current code this is more of a cleanup than an actual problem anyone would encounter.

Since I don't think it is possible with the current drivers to actually reach this error path I forced it by changing:

hid_attach+0x1e5:               call   -0x157afa        <usb_ep_xdescr_fill>
hid_attach+0x1ea:               testl  %eax,%eax

hid_attach+0x1ec:               jne    +0x1f6   <hid_attach+0x3e8>

to:

hid_attach+0x1e5:               call   -0x157afa        <usb_ep_xdescr_fill>
hid_attach+0x1ea:               testl  %eax,%eax

hid_attach+0x1ec:               je     +0x1f6   <hid_attach+0x3e8>

with mdb like this:

mdb -kw

> hid_attach+0x1ec/X
hid_attach+0x1ec:               1f6850f         
> hid_attach+0x1ec/W 1f6840f
hid_attach+0x1ec:               0x1f6850f       =       0x1f6840f

Plugged in a USB Device and got:

panic[cpu3]/thread=ffffff008d5d8c40: 
recursive mutex_enter, lp=ffffff1c914afb88 owner=ffffff008d5d8c40 thread=ffffff008d5d8c40


ffffff008d5d8720 unix:mutex_panic+58 ()
ffffff008d5d8790 unix:mutex_vector_enter+2b7 ()
ffffff008d5d87e0 hid:hid_detach_cleanup+98 ()
ffffff008d5d8880 hid:hid_attach+40d ()
ffffff008d5d88f0 genunix:devi_attach+92 ()
ffffff008d5d8930 genunix:attach_node+a7 ()
ffffff008d5d8980 genunix:i_ndi_config_node+7d ()
ffffff008d5d89b0 genunix:i_ddi_attachchild+48 ()
ffffff008d5d89f0 genunix:devi_attach_node+5e ()
ffffff008d5d8a70 genunix:config_immediate_children+bf ()
ffffff008d5d8af0 genunix:ndi_busop_bus_config+a4 ()
ffffff008d5d8b80 usb_mid:usb_mid_bus_config+b8 ()
ffffff008d5d8bd0 genunix:devi_config_common+a5 ()
ffffff008d5d8c20 genunix:mt_config_thread+58 ()
ffffff008d5d8c30 unix:thread_start+8 ()

With this fix and the mdb hack usb devices fail to attach but the system does not panic.

Without the mdb hack and this fix I tested plugging in a usb keyboard and it still worked as expected.

andy_js
  1. Ship It!
  2. 
      
rm
  1. Looks good. Probably my bug as well. Thanks.

  2. 
      
yuripv
  1. Just wondering how did you spot this -- just reading the code or using some code analyser?

    1. just reading the code looking for a different issue

  2. 
      
wiedi
Review request changed

Status: Closed (submitted)

Loading...