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

[Umang Jain]

Hi Laurent,

Thanks for the fix!

On 6/28/21 12:14 PM, 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: Umang Jain <umang.jain at ideasonboard.com>

Tested-by: Umang Jain <umang.jain 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_; }