Multiple Unlocks of a Critical Resource

Description

Multiple Unlocks of a Critical Resource is a concurrency vulnerability where software unlocks a critical resource more times than intended, leading to an unexpected state in the system. In concurrent environments, repeatedly unlocking a resource has unpredictable consequences depending on the lock type. With counting semaphores, extra unlock (post/signal) operations increase the available count beyond the intended maximum, potentially allowing more threads to access the resource simultaneously than intended. With mutexes, unlocking an already-unlocked mutex is typically undefined behavior that can corrupt internal lock state or crash the program.

Risk

Multiple unlocks create serious concurrency and security issues. With semaphores, artificially inflated counts can allow more concurrent access than the resource can safely handle, leading to data corruption or race conditions. With mutexes, the behavior varies by implementation—some crash, some silently corrupt state, some allow other threads to acquire the lock prematurely. This can break mutual exclusion guarantees, allowing concurrent access to critical sections that should be serialized. Attackers who can trigger multiple unlocks may exploit this to bypass synchronization controls, access shared resources without proper locking, or cause denial of service.

Solution

Ensure all control paths have exactly matching lock and unlock pairs. Use RAII patterns with lock guards that automatically handle unlocking. Track lock state explicitly if needed. Never unlock a resource that the current thread doesn't hold. Use static analysis tools to verify lock/unlock pairing. In error handling paths, be especially careful not to unlock resources that weren't successfully locked. Consider using try-finally or similar patterns to ensure unlock happens exactly once. Document locking protocols and review concurrency code carefully.

Common Consequences

Impact	Details
Availability	Scope: Availability DoS: Crash, Exit, or Restart - Unlocking non-held locks may crash or cause undefined behavior.
Integrity	Scope: Integrity Modify Memory - Corrupted lock state may allow unauthorized memory access.
Integrity	Scope: Integrity Unexpected State - System enters undefined state when unlock count exceeds lock count.

Example Code

Vulnerable Code

// Vulnerable: Semaphore unlocked more than locked
#include <semaphore.h>

sem_t resource_sem;

void vulnerable_semaphore_unlock(int error_occurred) {
    sem_wait(&resource_sem);  // Lock once

    if (error_occurred) {
        // Unlock in error path
        sem_post(&resource_sem);
        // ... error handling ...
    }

    process_resource();

    // Vulnerable: Unlocks again unconditionally
    sem_post(&resource_sem);  // Double unlock if error_occurred!

    // Semaphore count now higher than it should be
}

// Vulnerable: Mutex unlocked when not held
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

void vulnerable_mutex_unlock(int skip_lock) {
    if (!skip_lock) {
        pthread_mutex_lock(&mutex);
    }

    // ... process ...

    // Vulnerable: Unlocks even if never locked
    pthread_mutex_unlock(&mutex);  // Undefined if skip_lock was true
}

// Vulnerable: Error path unlocks, then normal path also unlocks
void vulnerable_double_unlock() {
    pthread_mutex_lock(&mutex);

    int result = do_operation();

    if (result < 0) {
        pthread_mutex_unlock(&mutex);  // Unlock on error
        handle_error();
        // Falls through!
    }

    // Vulnerable: Unconditional unlock
    pthread_mutex_unlock(&mutex);  // Double unlock if error occurred
}

// Vulnerable: Exception handling causes double unlock
#include <mutex>

std::mutex mtx;

class VulnerableUnlock {
public:
    void vulnerable_exception() {
        mtx.lock();

        try {
            riskyOperation();
        } catch (...) {
            mtx.unlock();  // Unlock in catch
            throw;  // Re-throw
        }

        mtx.unlock();  // Also unlocks here if no exception

        // If exception thrown after catch's unlock but before
        // re-throw propagates, mutex may be double-unlocked
    }

    // Vulnerable: Multiple return paths with inconsistent unlocking
    int vulnerable_returns() {
        mtx.lock();

        if (condition_a()) {
            mtx.unlock();
            return 1;
        }

        if (condition_b()) {
            // Forgot to unlock here
            return 2;  // Lock leaked
        }

        mtx.unlock();
        mtx.unlock();  // Vulnerable: Extra unlock if reached normally!
        return 0;
    }
};

// Vulnerable: Unlock in destructor when may not hold lock
class VulnerableLockHolder {
private:
    pthread_mutex_t* mutex;
    bool locked;

public:
    VulnerableLockHolder(pthread_mutex_t* m) : mutex(m), locked(false) {
        pthread_mutex_lock(mutex);
        locked = true;
    }

    void release() {
        pthread_mutex_unlock(mutex);
        // Vulnerable: Doesn't set locked = false
    }

    ~VulnerableLockHolder() {
        // Vulnerable: May unlock again if release() was called
        pthread_mutex_unlock(mutex);
    }
};

// Vulnerable: CVE-2009-0935 pattern - invalid address causes double unlock
void vulnerable_invalid_address(void* user_addr) {
    pthread_mutex_lock(&mutex);

    // Function fails when given invalid address
    int result = read_from_address(user_addr);

    if (result < 0) {
        // Error handler unlocks
        pthread_mutex_unlock(&mutex);

        // But error path continues and unlocks again!
    }

    // ... more processing ...

    pthread_mutex_unlock(&mutex);  // Double unlock on error
}

// Vulnerable: Java ReentrantLock unlocked more than locked
import java.util.concurrent.locks.ReentrantLock;

public class VulnerableUnlock {

    private final ReentrantLock lock = new ReentrantLock();

    public void vulnerableUnbalanced() {
        lock.lock();

        try {
            process();
        } finally {
            lock.unlock();
            lock.unlock();  // Vulnerable: Extra unlock throws IllegalMonitorStateException
        }
    }

    public void vulnerableConditionalUnlock(boolean wasLocked) {
        if (wasLocked) {
            lock.lock();
        }

        try {
            process();
        } finally {
            // Vulnerable: Unlocks even if never locked
            lock.unlock();  // Throws if wasLocked was false
        }
    }
}

Fixed Code

// Fixed: Single unlock with proper control flow
#include <semaphore.h>

sem_t resource_sem;

void fixed_semaphore_unlock(int error_occurred) {
    sem_wait(&resource_sem);

    if (error_occurred) {
        // Handle error without unlocking here
        handle_error();
        // Fall through to single unlock point
    } else {
        process_resource();
    }

    // Fixed: Single unlock point
    sem_post(&resource_sem);
}

// Alternative: Early return with unlock
void fixed_early_return(int error_occurred) {
    sem_wait(&resource_sem);

    if (error_occurred) {
        sem_post(&resource_sem);  // Unlock
        handle_error();
        return;  // Exit function, don't continue
    }

    process_resource();
    sem_post(&resource_sem);  // Only reached if no error
}

// Fixed: Track lock state explicitly
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

void fixed_track_state(int skip_lock) {
    int holding_lock = 0;

    if (!skip_lock) {
        pthread_mutex_lock(&mutex);
        holding_lock = 1;
    }

    // ... process ...

    // Fixed: Only unlock if we hold the lock
    if (holding_lock) {
        pthread_mutex_unlock(&mutex);
    }
}

// Fixed: Proper error handling without double unlock
void fixed_error_handling() {
    pthread_mutex_lock(&mutex);

    int result = do_operation();

    if (result < 0) {
        handle_error();
        // Don't unlock here, fall through to single unlock
    }

    // Fixed: Single unlock point for all paths
    pthread_mutex_unlock(&mutex);
}

// Fixed: Use RAII with lock_guard
#include <mutex>

std::mutex mtx;

class FixedUnlock {
public:
    void fixed_raii() {
        std::lock_guard<std::mutex> guard(mtx);

        riskyOperation();  // If throws, guard destructor unlocks once

        // Single automatic unlock when guard goes out of scope
    }

    // Fixed: unique_lock for manual control
    int fixed_manual() {
        std::unique_lock<std::mutex> lock(mtx);

        if (condition_a()) {
            lock.unlock();  // Marks as unlocked
            return 1;
        }

        if (condition_b()) {
            // Lock still held, will unlock in destructor
            return 2;
        }

        // Destructor handles unlock
        return 0;
    }
};

// Fixed: Proper lock holder class
class FixedLockHolder {
private:
    pthread_mutex_t* mutex;
    bool locked;

public:
    FixedLockHolder(pthread_mutex_t* m) : mutex(m), locked(false) {
        pthread_mutex_lock(mutex);
        locked = true;
    }

    void release() {
        if (locked) {
            pthread_mutex_unlock(mutex);
            locked = false;  // Track state
        }
    }

    ~FixedLockHolder() {
        // Fixed: Only unlock if still holding
        if (locked) {
            pthread_mutex_unlock(mutex);
        }
    }

    // Prevent copying
    FixedLockHolder(const FixedLockHolder&) = delete;
    FixedLockHolder& operator=(const FixedLockHolder&) = delete;
};

// Fixed: Proper validation without double unlock
void fixed_invalid_address(void* user_addr) {
    // Validate address before locking
    if (!is_valid_address(user_addr)) {
        handle_error();
        return;
    }

    pthread_mutex_lock(&mutex);

    int result = read_from_address(user_addr);

    if (result < 0) {
        // Error handling without unlock
        log_error();
    }

    // Fixed: Single unlock point
    pthread_mutex_unlock(&mutex);
}

// Fixed: Java with proper lock handling
import java.util.concurrent.locks.ReentrantLock;

public class FixedUnlock {

    private final ReentrantLock lock = new ReentrantLock();

    public void fixedTryFinally() {
        lock.lock();
        try {
            process();
        } finally {
            lock.unlock();  // Exactly one unlock
        }
    }

    public void fixedConditionalLock(boolean needsLock) {
        boolean acquired = false;

        if (needsLock) {
            lock.lock();
            acquired = true;
        }

        try {
            process();
        } finally {
            // Fixed: Only unlock if we acquired
            if (acquired) {
                lock.unlock();
            }
        }
    }

    // Alternative: tryLock pattern
    public void fixedTryLock() {
        if (lock.tryLock()) {
            try {
                process();
            } finally {
                lock.unlock();
            }
        } else {
            handleLockNotAcquired();
        }
    }
}

---

CVE Examples

• CVE-2009-0935: Attacker provided invalid address to memory-reading function, causing mutex to unlock twice.
• CVE-2017-15265: Double unlock in Linux kernel ALSA sequencer led to use-after-free.

---

References

1. MITRE Corporation. "CWE-765: Multiple Unlocks of a Critical Resource." https://cwe.mitre.org/data/definitions/765.html
2. CERT C Coding Standard. "CON31-C. Do not destroy a mutex while it is locked."
3. CERT C Coding Standard. "POS48-C. Do not unlock or destroy another POSIX thread's mutex."