[libcamera-devel] [PATCH] android: camera_device: Check capture_request validity

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Dec 01, 2020 at 05:14:45PM +0100, Jacopo Mondi wrote:
> Make sure the 'camera3_capture_request_t *' provided to
> CameraDevice::processCaptureRequest() is valid before attempting to
> access it.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> This patch fixes cros_camera_test:
> Camera3FrameTest/Camera3InvalidRequestTest.NullOrUnconfiguredRequest/*

Maybe this should be included in the commit message ?

> ---
> 
>  src/android/camera_device.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 4eb05df0fdc2..3c8205a095ae 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1398,6 +1398,11 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer
> 
>  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
>  {
> +	if (!camera3Request) {
> +		LOG(HAL, Error) << "Invalid capture request";

I'd say "No capture request provided" as it's more than invalid :-) Up
to you.

I wonder if the cros_camera_test case makes sense. There are lots of
ways to exercise APIs in ways that are completely against API contracts,
it would be pointless to check them all. This isn't about parsing a file
or network data from untrusted sources, we can rely on the camera
service doing its job more or less properly. We can't really influence
the tests much though, and this fixes an issue, so

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

> +		return -EINVAL;
> +	}
> +
>  	if (!camera3Request->num_output_buffers) {
>  		LOG(HAL, Error) << "No output buffers provided";
>  		return -EINVAL;

-- 
Regards,

Laurent Pinchart