[libcamera-devel] [PATCH] libcamera: camera: Ensure requests belong to the camera

[Kieran Bingham]

Quoting Laurent Pinchart (2022-03-29 22:58:05)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Mar 17, 2022 at 01:17:26PM +0000, Kieran Bingham via libcamera-devel wrote:
> > Requests are created by a Camera, and can only be queued
> > to that specific Camera. Enforce this during the public API
> > to prevent mis-use by incorrect applications.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/libcamera/camera.cpp | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index bb856d606f4a..dd6552e83eee 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -23,6 +23,7 @@
> >  #include "libcamera/internal/camera_controls.h"
> >  #include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/request.h"
> >  
> >  /**
> >   * \file libcamera/camera.h
> > @@ -1119,6 +1120,12 @@ int Camera::queueRequest(Request *request)
> >       if (ret < 0)
> >               return ret;
> >  
> > +     /* Requests can only be queued to the camera that created them.*/
> > +     if (request->_d()->camera() != this) {
> > +             LOG(Camera, Error) << "Request was not created by this camera";
> > +             return -EINVAL;
> > +     }
> 
> This could even be worth an ASSERT(), up to you.

If this was the other direction, such as a libcamera request getting
completed on the wrong camera, I would agree, that would be a fatal /
assert - but this check is on the incoming public API - where I think
it's better to just reject an invalid input.

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

Thanks

> 
> > +
> >       /*
> >        * The camera state may change until the end of the function. No locking
> >        * is however needed as PipelineHandler::queueRequest() will handle
> 
> -- 
> Regards,
> 
> Laurent Pinchart