[libcamera-devel] [PATCH] android: Fix improper file descriptor enumeration

[Laurent Pinchart]

Hi Nicholas,

Thank you for the patch.

On Sat, Nov 19, 2022 at 11:06:29PM -0600, Nicholas Roth via libcamera-devel wrote:
> Currently, they way camera_device.cpp handles file descriptors in a

s/they/the/

> buffer_handle_t is incorrect. This can result in the HAL attempting to
> treat uninitialized memory like file descriptors, usually resulting in
> a crash.
> 
> This patch brings the behavior of camera_device.cpp in line with how
> CameraBuffer handles file descriptors and planes.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=172

I've replied to the bug report in
https://bugs.libcamera.org/show_bug.cgi?id=172#c5. I'd like to
investigate this further, as numFds != buf.numPlanes() doesn't sound
right.

> Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
> ---
>  src/android/camera_device.cpp | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b20e389b..1459d582 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -781,14 +781,18 @@ CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer,
>  		return nullptr;
>  	}
>  
> +	if (camera3buffer->numFds > 1) {
> +		LOG(HAL, Fatal) << "Discontiguous planes are not supported";
> +	}

This will break operation on Chrome OS.

> +
> +	SharedFD fd{ camera3buffer->data[0] };
> +	if (!fd.isValid()) {
> +		LOG(HAL, Fatal) << "No valid fd";
> +		return nullptr;
> +	}
> +
>  	std::vector<FrameBuffer::Plane> planes(buf.numPlanes());
>  	for (size_t i = 0; i < buf.numPlanes(); ++i) {
> -		SharedFD fd{ camera3buffer->data[i] };
> -		if (!fd.isValid()) {
> -			LOG(HAL, Fatal) << "No valid fd";
> -			return nullptr;
> -		}
> -
>  		planes[i].fd = fd;
>  		planes[i].offset = buf.offset(i);
>  		planes[i].length = buf.size(i);

-- 
Regards,

Laurent Pinchart