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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Nov 2016 22:58:00 +0100
From:   Iago Abal <iago.abal@...il.com>
To:     Kalle Valo <kvalo@...eaurora.org>
Cc:     Solomon Peachy <pizza@...ftnet.org>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org
Subject: Potential deadlock BUG in drivers/net/wireless/st/cw1200/sta.c (Linux 4.9)

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