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

[paul.elder at ideasonboard.com]

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>

> ---
>  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
>