linux-kernel - Re: [PATCH] media: videobuf2: forbid create_bufs/remove

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] [day] [month] [year] [list]

Message-ID: <0f159e72-e5e4-44ea-8b3e-1ccb9341a9e8@samsung.com>
Date: Mon, 20 Oct 2025 18:05:17 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Benjamin Gaignard <benjamin.gaignard@...labora.com>, Hans Verkuil
	<hverkuil+cisco@...nel.org>, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: Tomasz Figa <tfiga@...omium.org>, Mauro Carvalho Chehab
	<mchehab@...nel.org>, Guennadi Liakhovetski <g.liakhovetski@....de>, Hans
	Verkuil <hverkuil@...nel.org>, stable@...r.kernel.org
Subject: Re: [PATCH] media: videobuf2: forbid create_bufs/remove_bufs when
 legacy fileio is active

On 20.10.2025 10:30, Benjamin Gaignard wrote:
> Le 20/10/2025 à 10:21, Marek Szyprowski a écrit :
>> On 20.10.2025 09:48, Benjamin Gaignard wrote:
>>> Le 20/10/2025 à 09:39, Hans Verkuil a écrit :
>>>> On 20/10/2025 09:34, Marek Szyprowski wrote:
>>>>> On 20.10.2025 09:11, Benjamin Gaignard wrote:
>>>>>> Le 16/10/2025 à 13:11, Marek Szyprowski a écrit :
>>>>>>> create_bufs and remove_bufs ioctl calls manipulate queue internal
>>>>>>> buffer
>>>>>>> list, potentially overwriting some pointers used by the legacy 
>>>>>>> fileio
>>>>>>> access mode. Simply forbid those calls when fileio is active to
>>>>>>> protect
>>>>>>> internal queue state between subsequent read/write calls.
>>>>>> Hi Marek,
>>>>>>
>>>>>> I may be wrong but using fileio API and create/remove API at the 
>>>>>> same
>>>>>> time
>>>>>> sound incorrect from application point of view, right ? If that not
>>>>>> the
>>>>>> case maybe we should also add a test in v4l2-compliance.
>>>>> Definitely that's incorrect and v4l2-core must forbid such calls. The
>>>>> standard reqbufs/qbuf/dqbuf API is also forbidden. Extending
>>>>> v4l2-compliance tools is probably a good idea.
>>>> Yes, please! A patch is welcome.
>>>>
>>>>    I also wonder if its a
>>>>> good time to add a kernel option to completely disable legacy fileio
>>>>> access mode, as it is not really needed for most of the systems
>>>>> nowadays.
>>>> No, that will break applications. Using read() is very common (and
>>>> convenient!)
>>>> for MPEG encoders such as the cx18 driver.
>>>>
>>>> The fileio code is not blocking any new development, it's just there
>>>> for those
>>>> drivers were it makes sense.
>>>>
>>>> Regards,
>>>>
>>>>      Hans
>>> I wonder if this patch in useful because when calling
>>> vb2_ioctl_create_bufs()
>>> it already check in vb2_verify_memory_type() if fileio is used or not.
>> Frankly speaking the original report I got was about mixing fileio with
>> vb2_ioctl_remove_bufs and that case is indeed not protected.
>>
>> While analyzing that I've inspected a symmetrical ioctl
>> (vb2_ioctl_create_bufs), but it looks I've I missed that a check is in
>> vb2_verify_memory_type(). I will remove it in v2 then.
>
> To keep vb2_ioctl_remove_bufs() symmetrical to vb2_ioctl_create_bufs()
> we should do in vb2_ioctl_remove_bufs() something like :
> res = vb2_verify_memory_type(vdev->queue, vdev->queue->memory, d->type);
> instead of vdev->queue->type != d->type.
>
> This way we test fileio too.


Right, that will fit best. I've sent a v2 with such change. Btw, the 
vb2_verify_memory_type() name is a bit misleading in this context. Maybe 
it should be renamed to something like vb2_is_queue_compatible()?


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland