[libcamera-devel] [PATCH] libcamera: thread: Fix locking when moving object

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2019-11-24 02:39:17 +0200, Laurent Pinchart wrote:
> When moving an Object to a Thread, messages posted for the object are
> move to the target thread's message queue. This requires locking the
> message queues of the current and target threads, as the target thread
> may (and is usually) running. The implementation is faulty as it locks
> the thread data instead of the message queue. This creates a race
> condition with a tiny but exploitable time window.
> 
> The issue was noticed by the event-thread test rarely but reproducibly
> failing with the following assertion error:
> 
> [1:39:33.850878042]FATAL default thread.cpp:440 assertion "data_ == receiver->thread()->data_" failed
> 
> The issue only occurred when libcamera was compiled in release mode,
> further hinting of a race condition.
> 
> Fixes: 01b930964acd ("libcamera: thread: Add a messaging passing API")
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/libcamera/thread.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp
> index e152af14631e..029a0e8fddd5 100644
> --- a/src/libcamera/thread.cpp
> +++ b/src/libcamera/thread.cpp
> @@ -456,8 +456,8 @@ void Thread::moveObject(Object *object)
>  	ThreadData *currentData = object->thread_->data_;
>  	ThreadData *targetData = data_;
>  
> -	MutexLocker lockerFrom(currentData->mutex_, std::defer_lock);
> -	MutexLocker lockerTo(targetData->mutex_, std::defer_lock);
> +	MutexLocker lockerFrom(currentData->messages_.mutex_, std::defer_lock);
> +	MutexLocker lockerTo(targetData->messages_.mutex_, std::defer_lock);
>  	std::lock(lockerFrom, lockerTo);
>  
>  	moveObject(object, currentData, targetData);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund