Double-lock bug in drivers/isdn/hardware/mISDN/hfcmulti.c

[Iago Abal]

Hi,

There is a potential double-lock sequence starting from hfc_remove_pci().

Forward trace:

  1. hfc_remove_pci()  LOCKS spin_lock_irqsave(, flags) at 5284
  2. hfc_remove_pci()  calls release_card(card) at 5285
  3. release_card()calls release_port(hc, hc->chan[ch].dch) at 4674
  4. release_port()calls plxsd_checksync(hc, 1) at 4595
  5. plxsd_checksync() calls hfcmulti_resync(hc, ..., rm)   at 1036 or 1044
  6. hfcmulti_resync() LOCKS spin_lock_irqsave(, flags) at 933

NB: Bug found by static analysis thanks to EBA
(https://github.com/IagoAbal/eba).

Hope it helps,

-- iago

Re: Potential deadlock BUG in drivers/net/wireless/st/cw1200/sta.c (Linux 4.9)

[Iago Abal]

Hi,

This still looks like a deadlock bug to me, could someone take a look
as well and confirm? I will help preparing a patch if needed.

Thanks,

-- iago


On Fri, Nov 18, 2016 at 10:58 PM, Iago Abal <iago.a...@gmail.com> wrote:
> Hi,
>
> With the help of a static bug finder (EBA -
> https://github.com/models-team/eba) I have found a potential deadlock
> in drivers/net/wireless/st/cw1200/
> sta.c. This happens due to a recursive mutex_lock on `priv->conf_mutex'.
>
> If this is indeed a bug, I will be happy to help with a patch.
>
> A quick (not elegant) fix could be to unlock before the call to
> `cw1200_do_unjoin' in line 1174, and lock again afterwards. It seems
> that `cw1200_join_complete' is always called with `priv->conf_mutex'
> held. Another option could be to add a Boolean parameter to
> `cw1200_do_unjoin' to choose whether this function should take the
> lock itself. Yet another option would be to have a
> `__cw1200_do_unjoin' that does not lock, and make `cw1200_do_unjoin' a
> wrapper over this that adds the locking; `cw1200_join_complete' would
> call `__cw1200_do_unjoin' instead.
>
> Someone who is actually familiar with this code may have a better
> proposal though.
>
> The trace is as follows:
>
> 1. Function `cw1200_join_complete_work' takes the first lock in line 1189:
>
> // see 
> https://github.com/torvalds/linux/blob/v4.9-rc5/drivers/net/wireless/st/cw1200/sta.c#L1189
> mutex_lock(& priv->conf_mutex);
>
> 2. and subsequently calls `cw1200_join_complete';
> 3. which calls `cw1200_do_unjoin' in line 1174;
> 4. and this latter function takes the lock for the second time in line 1387:
>
> // see 
> https://github.com/torvalds/linux/blob/v4.9-rc5/drivers/net/wireless/st/cw1200/sta.c#L1387
> mutex_lock(& priv->conf_mutex);
>
> Hope it helps!
>
> --
> iago

Potential deadlock BUG in drivers/net/wireless/st/cw1200/sta.c (Linux 4.9)

[Iago Abal]

Hi,

With the help of a static bug finder (EBA -
https://github.com/models-team/eba) I have found a potential deadlock
in drivers/net/wireless/st/cw1200/
sta.c. This happens due to a recursive mutex_lock on `priv->conf_mutex'.

If this is indeed a bug, I will be happy to help with a patch.

A quick (not elegant) fix could be to unlock before the call to
`cw1200_do_unjoin' in line 1174, and lock again afterwards. It seems
that `cw1200_join_complete' is always called with `priv->conf_mutex'
held. Another option could be to add a Boolean parameter to
`cw1200_do_unjoin' to choose whether this function should take the
lock itself. Yet another option would be to have a
`__cw1200_do_unjoin' that does not lock, and make `cw1200_do_unjoin' a
wrapper over this that adds the locking; `cw1200_join_complete' would
call `__cw1200_do_unjoin' instead.

Someone who is actually familiar with this code may have a better
proposal though.

The trace is as follows:

1. Function `cw1200_join_complete_work' takes the first lock in line 1189:

// see 
https://github.com/torvalds/linux/blob/v4.9-rc5/drivers/net/wireless/st/cw1200/sta.c#L1189
mutex_lock(& priv->conf_mutex);

2. and subsequently calls `cw1200_join_complete';
3. which calls `cw1200_do_unjoin' in line 1174;
4. and this latter function takes the lock for the second time in line 1387:

// see 
https://github.com/torvalds/linux/blob/v4.9-rc5/drivers/net/wireless/st/cw1200/sta.c#L1387
mutex_lock(& priv->conf_mutex);

Hope it helps!

--
iago

Potential double mutex_lock bug in net/ceph/auth.c

[Iago Abal]

Hi,

I'm testing a static bug finder (EBA) on Linux 4.7 release candidates
and I may have found a potential double lock:

  Double lock in net/ceph/auth.c
  second lock at 108: mutex_lock(& ac->mutex); [ceph_auth_build_hello]
  after calling from 263: ret = ceph_auth_build_hello(ac, msg_buf, msg_len);
  if ! ac->protocol -> true at 262
  first lock at 261: mutex_lock(& ac->mutex); [ceph_build_auth]

This seems to have been introduced by commit e9966076cdd9 ("libceph:
wrap auth methods in a mutex").

I hope it helps!

Thanks,

-- iago