[libcamera-devel,2/2] android: Check if Stream configurations were generated correctly
diff mbox series

Message ID 20211001161100.1405576-2-javierm@redhat.com
State Accepted
Headers show
Series
  • [libcamera-devel,1/2] gstreamer: Check if Stream configurations were generated correctly
Related show

Commit Message

Javier Martinez Canillas Oct. 1, 2021, 4:11 p.m. UTC 
The libcamera Android Camera HAL generates camera configurations for the
StillCapture, Raw and ViewFinder stream roles. But it's only checked for
the first one if the function succeeded.

This could lead to a NULL pointer deference if a pipeline handler failed
to generate a default configuration for one the other two stream roles.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 src/android/camera_capabilities.cpp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Oct. 11, 2021, 12:17 p.m. UTC | #1 
Hi Javier,

On Fri, Oct 01, 2021 at 06:11:00PM +0200, Javier Martinez Canillas wrote:
> The libcamera Android Camera HAL generates camera configurations for the
> StillCapture, Raw and ViewFinder stream roles. But it's only checked for
> the first one if the function succeeded.
>
> This could lead to a NULL pointer deference if a pipeline handler failed
> to generate a default configuration for one the other two stream roles.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> ---
>
>  src/android/camera_capabilities.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 87a6e1c6f26..baeedc11500 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -408,6 +408,11 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>  	std::vector<Size> supportedResolutions;
>  	std::unique_ptr<CameraConfiguration> cameraConfig =
>  		camera_->generateConfiguration({ StreamRole::Viewfinder });
> +	if (!cameraConfig) {
> +		LOG(HAL, Error) << "Failed to get supported YUV resolutions";
> +		return supportedResolutions;
> +	}
> +
>  	StreamConfiguration &cfg = cameraConfig->at(0);
>
>  	for (const Size &res : resolutions) {
> @@ -431,11 +436,17 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>  std::vector<Size>
>  CameraCapabilities::initializeRawResolutions(const PixelFormat &pixelFormat)
>  {
> +	std::vector<Size> supportedResolutions;
>  	std::unique_ptr<CameraConfiguration> cameraConfig =
>  		camera_->generateConfiguration({ StreamRole::Raw });
> +	if (!cameraConfig) {
> +		LOG(HAL, Error) << "Failed to get supported Raw resolutions";
> +		return supportedResolutions;
> +	}
> +
>  	StreamConfiguration &cfg = cameraConfig->at(0);
>  	const StreamFormats &formats = cfg.formats();
> -	std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
> +	supportedResolutions = formats.sizes(pixelFormat);
>
>  	return supportedResolutions;
>  }
> --
> 2.31.1
>
Laurent Pinchart Oct. 12, 2021, 2:39 a.m. UTC | #2 
Hi Javier,

Thank you for the patch.

On Fri, Oct 01, 2021 at 06:11:00PM +0200, Javier Martinez Canillas wrote:
> The libcamera Android Camera HAL generates camera configurations for the
> StillCapture, Raw and ViewFinder stream roles. But it's only checked for
> the first one if the function succeeded.
> 
> This could lead to a NULL pointer deference if a pipeline handler failed

s/deference/dereference/

I'll fix this when applying.

> to generate a default configuration for one the other two stream roles.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

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

> ---
> 
>  src/android/camera_capabilities.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 87a6e1c6f26..baeedc11500 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -408,6 +408,11 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>  	std::vector<Size> supportedResolutions;
>  	std::unique_ptr<CameraConfiguration> cameraConfig =
>  		camera_->generateConfiguration({ StreamRole::Viewfinder });
> +	if (!cameraConfig) {
> +		LOG(HAL, Error) << "Failed to get supported YUV resolutions";
> +		return supportedResolutions;
> +	}
> +
>  	StreamConfiguration &cfg = cameraConfig->at(0);
>  
>  	for (const Size &res : resolutions) {
> @@ -431,11 +436,17 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>  std::vector<Size>
>  CameraCapabilities::initializeRawResolutions(const PixelFormat &pixelFormat)
>  {
> +	std::vector<Size> supportedResolutions;
>  	std::unique_ptr<CameraConfiguration> cameraConfig =
>  		camera_->generateConfiguration({ StreamRole::Raw });
> +	if (!cameraConfig) {
> +		LOG(HAL, Error) << "Failed to get supported Raw resolutions";
> +		return supportedResolutions;
> +	}
> +
>  	StreamConfiguration &cfg = cameraConfig->at(0);
>  	const StreamFormats &formats = cfg.formats();
> -	std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
> +	supportedResolutions = formats.sizes(pixelFormat);
>  
>  	return supportedResolutions;
>  }

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 87a6e1c6f26..baeedc11500 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -408,6 +408,11 @@  CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
 	std::vector<Size> supportedResolutions;
 	std::unique_ptr<CameraConfiguration> cameraConfig =
 		camera_->generateConfiguration({ StreamRole::Viewfinder });
+	if (!cameraConfig) {
+		LOG(HAL, Error) << "Failed to get supported YUV resolutions";
+		return supportedResolutions;
+	}
+
 	StreamConfiguration &cfg = cameraConfig->at(0);
 
 	for (const Size &res : resolutions) {
@@ -431,11 +436,17 @@  CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
 std::vector<Size>
 CameraCapabilities::initializeRawResolutions(const PixelFormat &pixelFormat)
 {
+	std::vector<Size> supportedResolutions;
 	std::unique_ptr<CameraConfiguration> cameraConfig =
 		camera_->generateConfiguration({ StreamRole::Raw });
+	if (!cameraConfig) {
+		LOG(HAL, Error) << "Failed to get supported Raw resolutions";
+		return supportedResolutions;
+	}
+
 	StreamConfiguration &cfg = cameraConfig->at(0);
 	const StreamFormats &formats = cfg.formats();
-	std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
+	supportedResolutions = formats.sizes(pixelFormat);
 
 	return supportedResolutions;
 }