Re: [PATCH] media: videobuf2: forbid create_bufs/remove_bufs when legacy fileio is active

[Marek Szyprowski <m.szyprowski@samsung.com>]

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.

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