Semmle 1.21
Skip to end of metadata
Go to start of metadata

Name: Lock may not be released

Description: A lock that is acquired one or more times without a matching number of unlocks may cause a deadlock.

ID: cpp/unreleased-lock

Kind: problem

Severity: error

Precision: low

Query: UnreleasedLock.ql
/**
 * @name Lock may not be released
 * @description A lock that is acquired one or more times without a
 *              matching number of unlocks may cause a deadlock.
 * @kind problem
 * @id cpp/unreleased-lock
 * @problem.severity error
 * @precision low
 * @tags security
 *       external/cwe/cwe-764
 *       external/cwe/cwe-833
 */
import cpp
import semmle.code.cpp.commons.Synchronization

predicate lockBlock(MutexType t, BasicBlock b, int locks) {
  locks = strictcount(int i | b.getNode(i) = t.getLockAccess())
}

predicate unlockBlock(MutexType t, BasicBlock b, int unlocks) {
  unlocks = strictcount(int i | b.getNode(i) = t.getUnlockAccess())
}

/**
 * Holds if there is a call to `lock` or `tryLock` on `t` in
 * `lockblock`, and `failblock` is the successor if it fails.
 */
predicate failedLock(MutexType t, BasicBlock lockblock, BasicBlock failblock) {
  exists (ControlFlowNode lock |
    lock = lockblock.getEnd() and
    lock = t.getLockAccess() and
    lock.getAFalseSuccessor() = failblock
  )
}

/**
 * Holds if `b` locks `t` a net `netlocks` times. For example, if `b`
 * locks `t` twice and unlocks `t` four times, then `netlocks` will be
 * `-2`.
 */
predicate lockUnlockBlock(MutexType t, BasicBlock b, int netlocks) {
  lockBlock(t, b, netlocks) and not unlockBlock(t, b, _) or
  exists (int unlocks |
    not lockBlock(t, b, _) and unlockBlock(t, b, unlocks) and netlocks = -unlocks
  ) or
  exists (int locks, int unlocks |
    lockBlock(t, b, locks) and unlockBlock(t, b, unlocks) and netlocks = locks - unlocks
  )
}

/**
 * Holds if there is a control flow path from `src` to `b` such that
 * on that path the net number of locks is `locks`, and `locks` is
 * positive.
 */
predicate blockIsLocked(MutexType t, BasicBlock src, BasicBlock b, int locks) {
  lockUnlockBlock(t, b, locks) and src = b and locks > 0
  or
  exists (BasicBlock pred, int predlocks, int curlocks, int failedlock |
    pred = b.getAPredecessor() |
    blockIsLocked(t, src, pred, predlocks) and
    ( if failedLock(t, pred, b) then failedlock = 1 else failedlock = 0 ) and // count a failed lock as an unlock so the net is zero
    ( not lockUnlockBlock(t, b, _) and curlocks = 0 or
      lockUnlockBlock(t, b, curlocks)
    ) and
    locks = predlocks + curlocks -failedlock and locks > 0 and
    locks < 10 // arbitrary bound to fail gracefully in case of locking in a loop
  )
}

from Function c, MutexType t, BasicBlock src, BasicBlock exit, FunctionCall lock
where // restrict results to those methods that actually attempt to unlock
      t.getUnlockAccess().getEnclosingFunction() = c
  and blockIsLocked(t, src, exit, _)
  and exit.getEnd() = c
  and lock = src.getANode()
  and lock = t.getLockAccess()
select
  lock, "This lock might not be unlocked or might be locked more times than it is unlocked."

When a thread acquires a lock it must make sure to unlock it again; failing to do so can lead to deadlocks. If a lock allows a thread to acquire it multiple times, for example std::recursive_mutex, then the number of locks must match the number of unlocks in order to fully release the lock.

Recommendation

The best way to ensure that locks are always unlocked is to use RAII (Resource Acquisition Is Initialization). That means acquiring the lock in the constructor of a class, and releasing it in its destructor. A local-scoped instance of that class will then be destroyed when it leaves scope, even if an exception is thrown, ensuring that the lock is released.

Example

In this example, the mutex may not be unlocked if the function returns early.

std::mutex mutex;
int fun() {
	mutex.lock();
	bool succeeded = doWork();
	if (!succeeded) {
		// BAD: this does not release the mutex
		return -1
	}
	mutex.unlock();
	return 1;
}

In this second example, we show a simple RAII wrapper class for std::mutex. Using this ensures that even in the case of the early return, the mutex is released.

class RAII_Mutex
{
	std::mutex lock;
public:
	RAII_Mutex(mutex m) : lock(m)
	{
		lock.lock();
	}

	~RAII_Mutex()
	{
		lock.unlock();
	}
};


std::mutex mutex;
int fun() {
	RAII_Mutex(mutex);

	bool succeeded = doWork();
	if (!succeeded) {
		// GOOD: the RAII_Mutex is destroyed, releasing the lock
		return -1
	}
	
	return 1;
}

References
  • Common Weakness Enumeration: CWE-764.
  • Common Weakness Enumeration: CWE-833.