Online Linux Driver Verification Service (alpha)

0032

RULE_ERROR

Locking a mutex twice or unlocking without prior locking

Summary

You should not acquire or release the same mutex twice in the same process. All locked mutexes should be unlocked at finalization.

Description

There are several things you should watch for:

1. Double-locking. Mutex-type object provides exclusive access to sections of executable code. Thus retrying to acquire the current mutex will inevitably lead to a deadlock-situation. Deadlocks should be avoided, hence no double locks should take place.

2. Releasing an unlocked mutex.

3. Leaving a mutex locked at finalization.

Mutex is a special case of semaphore, restricted with just one possible requestor. However, it has its own wrapping functions. To initialize a mutex, macros are often used: DECLARE_MUTEX(name) and DECLARE_MUTEX_LOCKED(name). Locking is provided via mutex_lock(), mutex_trylock() and mutex_lock_interruptible() functions; unlocking--via mutex_unlock().

Links:

Sample bugfix

Example

Sample error (2.6.23 -> drivers/edac/edac_device.c):

void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev, unsigned long value)
{
    mutex_lock(&device_ctls_mutex); /*acquire the device_ctls_mutex*/

    /* cancel the current workq request */
    edac_device_workq_teardown(edac_dev); /* function acquires the lock device_ctls_mutex */
    /* leads to a double lock */

    /* restart the workq request, with new delay value */
    edac_device_workq_setup(edac_dev, value);
    ...

The solution to the problem outlined in the comments above is to acquire the mutex later, after the call to the function edac_device_workq_teardown().

Fixed version:

void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev, unsigned long value)
{

    /* cancel the current workq request, without the mutex lock */
    edac_device_workq_teardown(edac_dev);

    /* acquire the mutex before doing the workq setup */
    mutex_lock(&device_ctls_mutex);

    /* restart the workq request, with new delay value */
    edac_device_workq_setup(edac_dev, value);
    ...
}