Skip to content

Use-after-free: delete this in CoreVideoSource::unsubscribe and CameraDevice::close #709

@grisutheguru

Description

@grisutheguru

Summary

Both CoreVideoSource::unsubscribe() and CameraDevice::close() use delete this while other threads may be waiting on or about to acquire the object's mutex, leading to use-after-free / access to a destroyed mutex.

Details

CoreVideoSource::unsubscribe (src/video/corevideosource.cpp:100-112)

When subscribers drops to 0 and deleteOnClose is true, the code does:

biglock.lock();
if (--subscribers == 0) {
    if (deleteOnClose) {
        biglock.unlock();
        delete this;   // destroys QMutex biglock
        return;
    }
}
biglock.unlock();

Meanwhile pushFrame() on another thread does:

if (stopped) return;
const QMutexLocker<QMutex> locker(&biglock); // could lock a destroyed mutex

CameraDevice::close (src/video/cameradevice.cpp:289-300)

Same pattern: atomic refcount decrement happens before openDeviceLock is acquired. Between reaching 0 and removing from openDevices, another thread calling CameraDevice::open() can find and return a pointer to the about-to-be-deleted device.

Suggested Fix

Replace delete this with QObject::deleteLater() to defer destruction to the event loop, or use std::shared_ptr / std::enable_shared_from_this for shared ownership.

For CameraDevice::close(), acquire openDeviceLock before decrementing refcount:

bool CameraDevice::close()
{
    const QMutexLocker lock(&openDeviceLock);
    if (--refcount > 0)
        return false;
    openDevices.remove(devName);
    lock.unlock();
    avformat_close_input(&context);
    delete this;
    return true;
}

Impact

Potential crash or memory corruption in multi-threaded video/camera scenarios.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions