[libcamera-devel] [PATCH] android: camera_device: Fix null pointer dereference

[Hirokazu Honda]

Hi Laurent,

On Mon, Jun 28, 2021 at 3:52 PM <paul.elder at ideasonboard.com> wrote:
>
> Hi Laurent,
>
> On Mon, Jun 28, 2021 at 09:44:50AM +0300, Laurent Pinchart wrote:
> > Commit 7532caa2c77b ("android: camera_device: Reset config_ if
> > Camera::configure() fails") reworked the configuration sequence to
> > ensure that the CameraConfiguration pointers gets reset when
> > configuration fails. This inadvertently causes a null pointer
> > dereference, as the CameraStream constructor accesses the camera
> > configuration through CameraDevice::cameraConfiguration() before the
> > internal config_ pointer is set.
> >
> > Fix this by passing the configuration pointer explicitly to the
> > CameraStream constructor.
> >
> > Fixes: 7532caa2c77b ("android: camera_device: Reset config_ if Camera::configure() fails")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Tested-by: Paul Elder <paul.elder at ideasonboard.com>

Oops, I couldn't catch this in the previous review. :(
Thanks for fixing.

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
>
> > ---
> >  src/android/camera_device.cpp | 4 ++--
> >  src/android/camera_device.h   | 4 ----
> >  src/android/camera_stream.cpp | 6 +++---
> >  src/android/camera_stream.h   | 3 ++-
> >  4 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 13ee5fab4412..678cde231c63 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -682,8 +682,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >               config->addConfiguration(streamConfig.config);
> >
> >               for (auto &stream : streamConfig.streams) {
> > -                     streams_.emplace_back(this, stream.type, stream.stream,
> > -                                           config->size() - 1);
> > +                     streams_.emplace_back(this, config.get(), stream.type,
> > +                                           stream.stream, config->size() - 1);
> >                       stream.stream->priv = static_cast<void *>(&streams_.back());
> >               }
> >       }
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 18cf51189e90..3361918d4484 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -48,10 +48,6 @@ public:
> >       unsigned int id() const { return id_; }
> >       camera3_device_t *camera3Device() { return &camera3Device_; }
> >       const std::shared_ptr<libcamera::Camera> &camera() const { return camera_; }
> > -     libcamera::CameraConfiguration *cameraConfiguration() const
> > -     {
> > -             return config_.get();
> > -     }
> >
> >       const std::string &maker() const { return maker_; }
> >       const std::string &model() const { return model_; }
> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> > index b2f03b505199..bf4a7b41a70a 100644
> > --- a/src/android/camera_stream.cpp
> > +++ b/src/android/camera_stream.cpp
> > @@ -39,10 +39,10 @@ LOG_DECLARE_CATEGORY(HAL)
> >   * and buffer allocation.
> >   */
> >
> > -CameraStream::CameraStream(CameraDevice *const cameraDevice, Type type,
> > +CameraStream::CameraStream(CameraDevice *const cameraDevice,
> > +                        CameraConfiguration *config, Type type,
> >                          camera3_stream_t *camera3Stream, unsigned int index)
> > -     : cameraDevice_(cameraDevice),
> > -       config_(cameraDevice->cameraConfiguration()), type_(type),
> > +     : cameraDevice_(cameraDevice), config_(config), type_(type),
> >         camera3Stream_(camera3Stream), index_(index)
> >  {
> >       if (type_ == Type::Internal || type_ == Type::Mapped) {
> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> > index 3401672233ca..8ecc6e345414 100644
> > --- a/src/android/camera_stream.h
> > +++ b/src/android/camera_stream.h
> > @@ -110,7 +110,8 @@ public:
> >               Internal,
> >               Mapped,
> >       };
> > -     CameraStream(CameraDevice *const cameraDevice, Type type,
> > +     CameraStream(CameraDevice *const cameraDevice,
> > +                  libcamera::CameraConfiguration *config, Type type,
> >                    camera3_stream_t *camera3Stream, unsigned int index);
> >
> >       Type type() const { return type_; }
> > --
> > Regards,
> >
> > Laurent Pinchart
> >