[libcamera-devel] [PATCH] pipeline: raspberrypi: Fix possible null dereference

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-05-22 18:34:15)
> Hi Naush,
> 
> Thank you for the patch.
> 
> On Fri, May 20, 2022 at 01:49:19PM +0100, Naushir Patuck via libcamera-devel wrote:
> > The freeBuffers() cleanup code calls into the IPA to unmap and free shared
> > buffers. However, this function could be called before the IPA has opened (via
> > registerCamera()), causing a segmentation fault. Fix this by guarding against
> > calling the IPA if it has not been opened.
> > 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 2636acb758b7..26cd4e5f2b99 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1484,10 +1484,12 @@ void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffer
> >  
> >  void RPiCameraData::freeBuffers()
> >  {
> > -     /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */
> > -     std::vector<unsigned int> ipaBuffers(ipaBuffers_.begin(), ipaBuffers_.end());
> > -     ipa_->unmapBuffers(ipaBuffers);
> > -     ipaBuffers_.clear();
> > +     if (ipa_) {
> > +             /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */
> > +             std::vector<unsigned int> ipaBuffers(ipaBuffers_.begin(), ipaBuffers_.end());
> 
> I'll wrap this when applying.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Can this be handled more generically by the interface layer between the
IPA? (Doesn't need to be done for this patch), but it seems like
something that could occur in other places (/pipelines) too.

--
Kieran

> 
> > +             ipa_->unmapBuffers(ipaBuffers);
> > +             ipaBuffers_.clear();
> > +     }
> >  
> >       for (auto const stream : streams_)
> >               stream->releaseBuffers();
> 
> -- 
> Regards,
> 
> Laurent Pinchart