[libcamera-devel] [PATCH] v4l2: Accept read-only buffers mappings and require MAP_SHARED

[Laurent Pinchart]

Hi Nejc,

On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:
> Tested-by: Nejc <galof.nejc at gmail.com>

I'll update this to

Tested-by: Nejc Galof <galof.nejc at gmail.com>

and add

Reported-by: Nejc Galof <galof.nejc at gmail.com>

Thank you for your first contribution to libcamera :-)

> V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof <galof.nejc at gmail.com>
> napisala:
> 
> > Hello.
> > Tomorrow I will test and send you an answer.
> >
> > Thank you
> > Nejc Galof
> >
> > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart <
> > laurent.pinchart at ideasonboard.com> napisala:
> >
> >> V4L2 is happy to map buffers read-only for capture devices (but rejects
> >> write-only mappings). We can support this as the dmabuf mmap()
> >> implementation supports it. This change fixes usage of the V4L2
> >> compatibility layer with OpenCV.
> >>
> >> While at it, attempt to validate the other flags. videobuf2 requires
> >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.
> >> While unlikly, other flags could get rejected by other kernel layers for
> >> V4L2 buffers but not for dmabuf. This can be handled later if the need
> >> arises.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> ---
> >>  src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--
> >>  1 file changed, 11 insertions(+), 2 deletions(-)
> >>
> >> Nejc, could you please test this patch ? It's slightly different than
> >> the one I've shared on the IRC channel. If you don't mind appearing in
> >> the git development history, you can reply with
> >>
> >> Tested-by: Name <email at address>
> >>
> >> and I'll add that (as well as a corresponding Reported-by tag) to the
> >> commit before pushing it.
> >>
> >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp
> >> b/src/v4l2/v4l2_camera_proxy.cpp
> >> index f3470a6d312a..ebe7601a4ba6 100644
> >> --- a/src/v4l2/v4l2_camera_proxy.cpp
> >> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,
> >> void *addr, size_t length,
> >>
> >>         MutexLocker locker(proxyMutex_);
> >>
> >> -       /* \todo Validate prot and flags properly. */
> >> -       if (prot != (PROT_READ | PROT_WRITE)) {
> >> +       /*
> >> +        * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject
> >> +        * PROT_EXEC as it makes no sense.
> >> +        */
> >> +       if (!(prot & PROT_READ) || prot & PROT_EXEC) {
> >> +               errno = EINVAL;
> >> +               return MAP_FAILED;
> >> +       }
> >> +
> >> +       /* videobuf2 also requires MAP_SHARED. */
> >> +       if (!(flags & MAP_SHARED)) {
> >>                 errno = EINVAL;
> >>                 return MAP_FAILED;
> >>         }
> >>
> >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440

-- 
Regards,

Laurent Pinchart