linux-kernel - Re: [PATCH v4 1/3] media: v4l2-dev.c: Add Meta: to the name of metadata devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221221221601.1636c763@sal.lan>
Date:   Wed, 21 Dec 2022 22:16:01 +0000
From:   Mauro Carvalho Chehab <mchehab@...nel.org>
To:     Ricardo Ribalda <ribalda@...omium.org>
Cc:     Yunke Cao <yunkec@...omium.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        linux-media@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/3] media: v4l2-dev.c: Add Meta: to the name of
 metadata devices

Em Fri, 02 Dec 2022 18:08:17 +0100
Ricardo Ribalda <ribalda@...omium.org> escreveu:

> Devices with Metadata output (like uvc), create two video devices, one
> for the data itself and another one for the metadata.
> 
> Add a "Meta: " to the beginning of the device name, as suggested by Mauro,
> to avoid having multiple devices with the same name.

Hmm... I don't remember when I suggested it, but I suspect that such
change could cause regressions on userspace.

I double-checked: the two apps I know that have code to detect meta
devices (Camorama and ZBar) won't be affected by name changes, as they
always open v4l2 devnodes before start seeing them as capture devices,
and run VIDIOC_QUERYCAP to check for V4L2_CAP_VIDEO_CAPTURE:

   if (first_device < 0) {
        fd = open(file, O_RDWR);
        if (fd < 0) {
            devices[n_devices].is_valid = FALSE;
        } else {
            if (ioctl(fd, VIDIOC_QUERYCAP, &vid_cap) == -1) {
                devices[n_devices].is_valid = FALSE;
            } else if (!(vid_cap.device_caps & V4L2_CAP_VIDEO_CAPTURE)) {
                devices[n_devices].is_valid = FALSE;
            } else {
                n_valid_devices++;
                devices[n_devices].is_valid = TRUE;
            }
        }

        close(fd);
    } else {
        devices[n_devices].is_valid = devices[first_device].is_valid;
    }

You need to double-check if this won't cause userspace regressions, specially
on browsers, as they have more complex logic, and seem to have already some
logic to detect and remove meta files.

> 
> Fixes v4l2-compliance:
> Media Controller ioctls:
>      fail: v4l2-test-media.cpp(205): v2_entity_names_set.find(key) != v2_entity_names_set.end()
>    test MEDIA_IOC_G_TOPOLOGY: FAIL
>      fail: v4l2-test-media.cpp(394): num_data_links != num_links
>    test MEDIA_IOC_ENUM_ENTITIES/LINKS: FAIL
> 
> Suggested-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 397d553177fa..5c2c9ebb6b96 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -901,6 +901,15 @@ int __video_register_device(struct video_device *vdev,
>  	if (WARN_ON(type != VFL_TYPE_SUBDEV && !vdev->device_caps))
>  		return -EINVAL;
>  
> +	/* Add Meta: to metadata device names */
> +	if (vdev->device_caps &
> +	    (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
> +		char aux[sizeof(vdev->name)];
> +
> +		snprintf(aux, sizeof(aux), "Meta: %s", vdev->name);
> +		strscpy(vdev->name, aux, sizeof(aux));
> +	}
> +
>  	/* v4l2_fh support */
>  	spin_lock_init(&vdev->fh_lock);
>  	INIT_LIST_HEAD(&vdev->fh_list);
>