[libcamera-devel] [PATCH] libcamera: Fix crash caused by reading uninitialised delayed controls

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Tue, Sep 28, 2021 at 04:36:34PM +0100, David Plowman wrote:
> This fixes bug https://bugs.libcamera.org/show_bug.cgi?id=74.

This should be moved before the Signed-off-by line, with

Bug: https://bugs.libcamera.org/show_bug.cgi?id=74.

> The cause is that we read out delayed values using a frame's sequence
> number (DelayedControls::get). But we fill the values up
> (DelayedControls::applyControls) incrementing writeCount by only one
> even if the sequence number has jumped by several since last
> time. This is exactly what happens when frames are being dropped.
> 
> So the fix is to increment writeCount by "as much as the sequence
> number has jumped since last time", which means that we just follow
> the sequence number directly.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/libcamera/delayed_controls.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
> index 90ce7e0b..9667187e 100644
> --- a/src/libcamera/delayed_controls.cpp
> +++ b/src/libcamera/delayed_controls.cpp
> @@ -279,7 +279,7 @@ void DelayedControls::applyControls(uint32_t sequence)
>  		}
>  	}
>  
> -	writeCount_++;
> +	writeCount_ = sequence - firstSequence_ + 1;

I'm always confused when I read this file, I think one day I'll dive
deep and do something about it :-) Until then, this looks good to me.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

A second pair of eyes would be nice (and testing on IPU3 too).

>  
>  	while (writeCount_ > queueCount_) {
>  		LOG(DelayedControls, Debug)

-- 
Regards,

Laurent Pinchart