[libcamera-devel] [PATCH 1/2] libcamera: Request: validate state on complete

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Mar 02, 2021 at 12:23:40PM +0000, Kieran Bingham wrote:
> Requests should only be completed from the RequestPending state.
> 
> Requests which are completed from the RequestCancelled, or RequestComplete
> state, will indicate that a double-complete has been called on the Request,
> or that it has been used internally after it has been given back to the
> application.
> 
> Ensure that this can be caught early if it occurs by enforcing the state
> required with an assert.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
> This issue has been seen while developing the IPA for the IPU3.
> 
> Requests are completed, and then completed again while closing.
> It is likely that this occured, where a request was completed, and was
> then re-used.
> 
> A use case where the Request was deleted instead of re-used was also
> seen which caused a segmentation fault instead, due to the use of the
> memory after free().
> 
>  src/libcamera/request.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index e561ce1d5d0e..24c3694de5fc 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -262,7 +262,9 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
>   */
>  void Request::complete()
>  {
> +	ASSERT(status_ == RequestPending);
>  	ASSERT(!hasPendingBuffers());
> +
>  	status_ = cancelled_ ? RequestCancelled : RequestComplete;
>  
>  	LOG(Request, Debug)

-- 
Regards,

Laurent Pinchart