[{"id":22070,"web_url":"https://patchwork.libcamera.org/comment/22070/","msgid":"","date":"2022-01-28T22:02:59","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":113,"url":"https://patchwork.libcamera.org/api/people/113/","name":"Nejc Galof","email":"galof.nejc@gmail.com"},"content":"Hello.\nTomorrow I will test and send you an answer.\n\nThank you\nNejc Galof\n\nV V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> napisala:\n\n> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> write-only mappings). We can support this as the dmabuf mmap()\n> implementation supports it. This change fixes usage of the V4L2\n> compatibility layer with OpenCV.\n>\n> While at it, attempt to validate the other flags. videobuf2 requires\n> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> While unlikly, other flags could get rejected by other kernel layers for\n> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> arises.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> 1 file changed, 11 insertions(+), 2 deletions(-)\n>\n> Nejc, could you please test this patch ? It's slightly different than\n> the one I've shared on the IRC channel. If you don't mind appearing in\n> the git development history, you can reply with\n>\n> Tested-by: Name \n>\n> and I'll add that (as well as a corresponding Reported-by tag) to the\n> commit before pushing it.\n>\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> b/src/v4l2/v4l2_camera_proxy.cpp\n> index f3470a6d312a..ebe7601a4ba6 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> void *addr, size_t length,\n>\n> MutexLocker locker(proxyMutex_);\n>\n> - /* \\todo Validate prot and flags properly. */\n> - if (prot != (PROT_READ | PROT_WRITE)) {\n> + /*\n> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> + * PROT_EXEC as it makes no sense.\n> + */\n> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> + errno = EINVAL;\n> + return MAP_FAILED;\n> + }\n> +\n> + /* videobuf2 also requires MAP_SHARED. */\n> + if (!(flags & MAP_SHARED)) {\n> errno = EINVAL;\n> return MAP_FAILED;\n> }\n>\n> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3C3A7BF415\n\tfor ;\n\tFri, 28 Jan 2022 22:13:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9646A609B5;\n\tFri, 28 Jan 2022 23:13:26 +0100 (CET)","from mail-il1-x12e.google.com (mail-il1-x12e.google.com\n\t[IPv6:2607:f8b0:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F11A604F1\n\tfor ;\n\tFri, 28 Jan 2022 23:03:12 +0100 (CET)","by mail-il1-x12e.google.com with SMTP id y17so6649532ilm.1\n\tfor ;\n\tFri, 28 Jan 2022 14:03:12 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"BrK0cyQw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=RMEdFJG+4C5+VKfaC1qIjprEo1cTolS2l6iD2V1WVV4=;\n\tb=BrK0cyQwg4qY+k4Vt514zNqM9HwsgKtbkFWu7ICcWBFBgk45/dwFwQS/lic61FTCjB\n\tBdUTNj0B8SyGu/uJuh0LSCbvy/xV7Ohf26jNFMGbRReqYnIU2kDYs46hcmy/5R+VtV4n\n\tjpWykDhfxNKv3Ng0UiPpvN9W6uSJjmDjBFrFXFv4lZ4JxdHnyaTgCY01O5SYcN2THqGk\n\tJIog4fURQkCq7GzbqMwOssj/j/KZRUOmPEQtadMJeLYrJ/COBLQ0Pj3CaH9EKj+bEcwz\n\txqNqKVR8VlNYrryd8PLO6ycNxkxoBnDsu52i+8oxbBPc9HBID/2/dghy3jn8DfJtCqbB\n\tfvDg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=RMEdFJG+4C5+VKfaC1qIjprEo1cTolS2l6iD2V1WVV4=;\n\tb=ki8M3+LCqna+aNplglI9qlG0+xvTWgiCiQ/lBarTbEPg858flEuASUdTJyZeefy+rg\n\t65o+MpIkdHtPYqTcCmAG0fy48QuiMUqPKert3R+NOnXRBaz9u5zjJYugMYvs4GYHLBsV\n\tMExTvWJ4owADe2AHPJ+nfza4tbWuS+bJy7SY5Vx4DRfkoxuLvLccFBSqnRv8ft8pD25J\n\tdk2tbcLdP/9tbXfnSQpDrsWsozZOf96EjzKRozs2VLFva6c5gW9tcUUJLhPff/JFbdZB\n\tB9iIpWQ9SP7mu/8D3q5zfl2ztg5UVhGwQb6R0/4HsPGQWq8kJ+avIaUfPY4VDERlkzsb\n\tXRiQ==","X-Gm-Message-State":"AOAM531gt+7JOeFq4lyi8LqHAZMx5zhRIVfY5ZXEUBk+nmPzXzszJH7Y\n\tZgt5pDFPm4hrNLW9Cq9wc7jwSZ6HaN+RV8ydGvI=","X-Google-Smtp-Source":"ABdhPJxIcPz8L+XRY5VhuVIqlnmo/H8WQ4ahhjjIC97wGrOBE6+V7ga2ZpOCR+Wj8U2eaocVhglS3874zsMXemG8PMk=","X-Received":"by 2002:a92:c091:: with SMTP id\n\th17mr7435623ile.204.1643407391133; \n\tFri, 28 Jan 2022 14:03:11 -0800 (PST)","MIME-Version":"1.0","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>","From":"Nejc Galof ","Date":"Fri, 28 Jan 2022 23:02:59 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000c5168d05d6ab99d0\"","X-Mailman-Approved-At":"Fri, 28 Jan 2022 23:13:25 +0100","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22071,"web_url":"https://patchwork.libcamera.org/comment/22071/","msgid":"","date":"2022-01-28T22:18:01","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":113,"url":"https://patchwork.libcamera.org/api/people/113/","name":"Nejc Galof","email":"galof.nejc@gmail.com"},"content":"Tested-by: Nejc \n\nV V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof \nnapisala:\n\n> Hello.\n> Tomorrow I will test and send you an answer.\n>\n> Thank you\n> Nejc Galof\n>\n> V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> napisala:\n>\n>> V4L2 is happy to map buffers read-only for capture devices (but rejects\n>> write-only mappings). We can support this as the dmabuf mmap()\n>> implementation supports it. This change fixes usage of the V4L2\n>> compatibility layer with OpenCV.\n>>\n>> While at it, attempt to validate the other flags. videobuf2 requires\n>> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n>> While unlikly, other flags could get rejected by other kernel layers for\n>> V4L2 buffers but not for dmabuf. This can be handled later if the need\n>> arises.\n>>\n>> Signed-off-by: Laurent Pinchart \n>> ---\n>> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n>> 1 file changed, 11 insertions(+), 2 deletions(-)\n>>\n>> Nejc, could you please test this patch ? It's slightly different than\n>> the one I've shared on the IRC channel. If you don't mind appearing in\n>> the git development history, you can reply with\n>>\n>> Tested-by: Name \n>>\n>> and I'll add that (as well as a corresponding Reported-by tag) to the\n>> commit before pushing it.\n>>\n>> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n>> b/src/v4l2/v4l2_camera_proxy.cpp\n>> index f3470a6d312a..ebe7601a4ba6 100644\n>> --- a/src/v4l2/v4l2_camera_proxy.cpp\n>> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n>> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n>> void *addr, size_t length,\n>>\n>> MutexLocker locker(proxyMutex_);\n>>\n>> - /* \\todo Validate prot and flags properly. */\n>> - if (prot != (PROT_READ | PROT_WRITE)) {\n>> + /*\n>> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n>> + * PROT_EXEC as it makes no sense.\n>> + */\n>> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n>> + errno = EINVAL;\n>> + return MAP_FAILED;\n>> + }\n>> +\n>> + /* videobuf2 also requires MAP_SHARED. */\n>> + if (!(flags & MAP_SHARED)) {\n>> errno = EINVAL;\n>> return MAP_FAILED;\n>> }\n>>\n>> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440\n>> --\n>> Regards,\n>>\n>> Laurent Pinchart\n>>\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 416F1BDCBF\n\tfor ;\n\tFri, 28 Jan 2022 22:18:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 61643609B8;\n\tFri, 28 Jan 2022 23:18:15 +0100 (CET)","from mail-il1-x12e.google.com (mail-il1-x12e.google.com\n\t[IPv6:2607:f8b0:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CA96604F1\n\tfor ;\n\tFri, 28 Jan 2022 23:18:14 +0100 (CET)","by mail-il1-x12e.google.com with SMTP id u5so6626687ilq.9\n\tfor ;\n\tFri, 28 Jan 2022 14:18:14 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"GWIvLwm0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ywhWM7BAPlFF0USU2HyHpkdBhN1X9LVYT4Er2evykAw=;\n\tb=GWIvLwm0x7DkLz9LfpBqCJEeTLmqqPrXLLlLbQxIu440imQfherhJ2IlTaOPQQIjrK\n\tujTHhiCqixjegQjg6Xt8EC1/ne8R+FRS7dDhAf0kT8ivDMUJY0gbMIsAZcdkzQEkgMxK\n\tbPrQxyhjunMVVBWp66b6Y7GSOOcaFaD2GbBvVCxm1rBdopR4qjtBgdlO95LbncG1Y38q\n\tB0p66ECVtEqfvJZW+YoiIgg8qG4405exF6uFHNXBK6XaitMz34ZqOCSSaXnTySxZN72e\n\tHWwnU29fVuh9JMrFXTdebJe9coIYXTxvhdN+j4q9B1Td+H7MFbVg4Oa0IL6I0ha6yCsv\n\tvKQw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ywhWM7BAPlFF0USU2HyHpkdBhN1X9LVYT4Er2evykAw=;\n\tb=vttClsVcbCMFVMS/uzs9PjaVqw6IFCT7K2ojjC4Cx/OFXF5VRs8jYBLQsifql2L+79\n\tScYMiNrKr3ccUAI11sIfOIuZKCqDpmob8UERhllvc1PmMGio20IJKfiRxRaW6CneP4BF\n\td2cbng6Bfgj0Sh4I0geV5cRaf4y4dqaFvieiDBnTx0/yKLA2VMDjB8gCsmSr9yL247DZ\n\tO6KvxrVH3IUHiSGbatIJoT708U11nPSrOBN9+OSiXPE2MiYpEf1bvodHcYSPgRdYHohH\n\t++gVLPMpYC8p+3K+gcIkPxCeaMI42Uwo2yaKlXbPHnZrHWh6uY4YSQvH672ukWjn+7Qc\n\tsrBg==","X-Gm-Message-State":"AOAM533zfXH44Z9gqfceKgm6rP1vlgKc33lN+UAuRbhiVPDWb3wHLvi/\n\t/PD6mCiTHu20pmV4SN18xSj5X7d//rJeRr96Gak=","X-Google-Smtp-Source":"ABdhPJyZ0/4hjH8xnaNwIAjMjzfhjzAqqC2tdpLQZwAiDxFr/SqYgWtf4leKUSw7+5l8d2igOw47CVAo2xOdZYxECbE=","X-Received":"by 2002:a92:2e0d:: with SMTP id\n\tv13mr7779339ile.303.1643408292856; \n\tFri, 28 Jan 2022 14:18:12 -0800 (PST)","MIME-Version":"1.0","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t","In-Reply-To":"","From":"Nejc Galof ","Date":"Fri, 28 Jan 2022 23:18:01 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000844e2a05d6abcfc2\"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22072,"web_url":"https://patchwork.libcamera.org/comment/22072/","msgid":"","date":"2022-01-28T22:27:47","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nejc,\n\nOn Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> Tested-by: Nejc \n\nI'll update this to\n\nTested-by: Nejc Galof \n\nand add\n\nReported-by: Nejc Galof \n\nThank you for your first contribution to libcamera :-)\n\n> V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof \n> napisala:\n> \n> > Hello.\n> > Tomorrow I will test and send you an answer.\n> >\n> > Thank you\n> > Nejc Galof\n> >\n> > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart <\n> > laurent.pinchart@ideasonboard.com> napisala:\n> >\n> >> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> >> write-only mappings). We can support this as the dmabuf mmap()\n> >> implementation supports it. This change fixes usage of the V4L2\n> >> compatibility layer with OpenCV.\n> >>\n> >> While at it, attempt to validate the other flags. videobuf2 requires\n> >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> >> While unlikly, other flags could get rejected by other kernel layers for\n> >> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> >> arises.\n> >>\n> >> Signed-off-by: Laurent Pinchart \n> >> ---\n> >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> >>\n> >> Nejc, could you please test this patch ? It's slightly different than\n> >> the one I've shared on the IRC channel. If you don't mind appearing in\n> >> the git development history, you can reply with\n> >>\n> >> Tested-by: Name \n> >>\n> >> and I'll add that (as well as a corresponding Reported-by tag) to the\n> >> commit before pushing it.\n> >>\n> >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> >> b/src/v4l2/v4l2_camera_proxy.cpp\n> >> index f3470a6d312a..ebe7601a4ba6 100644\n> >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> >> void *addr, size_t length,\n> >>\n> >> MutexLocker locker(proxyMutex_);\n> >>\n> >> - /* \\todo Validate prot and flags properly. */\n> >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> >> + /*\n> >> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> >> + * PROT_EXEC as it makes no sense.\n> >> + */\n> >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> >> + errno = EINVAL;\n> >> + return MAP_FAILED;\n> >> + }\n> >> +\n> >> + /* videobuf2 also requires MAP_SHARED. */\n> >> + if (!(flags & MAP_SHARED)) {\n> >> errno = EINVAL;\n> >> return MAP_FAILED;\n> >> }\n> >>\n> >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 77B18BF415\n\tfor ;\n\tFri, 28 Jan 2022 22:28:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBD1D609B9;\n\tFri, 28 Jan 2022 23:28:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 77264609B2\n\tfor ;\n\tFri, 28 Jan 2022 23:28:09 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCE9F471;\n\tFri, 28 Jan 2022 23:28:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tP1a9tpt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643408889;\n\tbh=+oDF2xalXmbShOgvgvjcfk17xuxFdxVuI682NayXcuc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tP1a9tptgdhu2HV9TxAZSyXLZhdYcayXi42h4W2S68Dk//D3s/g/BVXqPBLA7t3T5\n\ts9J4JxwPp0wRRBVtWyYi/AOLZOUunsEjXiAzcSa3lTeTcrsaICCmQW2COPozrPmpH9\n\trQmcJ/Zjb1ywhR8cFnTbv8ZypuzFYtdfcutILYmQ=","Date":"Sat, 29 Jan 2022 00:27:47 +0200","From":"Laurent Pinchart ","To":"Nejc Galof ","Message-ID":"","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22073,"web_url":"https://patchwork.libcamera.org/comment/22073/","msgid":"<164348897616.1778844.4614155599174754915@Monstersaurus>","date":"2022-01-29T20:42:56","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-01-28 22:27:47)\n> Hi Nejc,\n> \n> On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> > Tested-by: Nejc \n> \n> I'll update this to\n> \n> Tested-by: Nejc Galof \n> \n> and add\n> \n> Reported-by: Nejc Galof \n> \n> Thank you for your first contribution to libcamera :-)\n> \n> > V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof \n> > napisala:\n> > \n> > > Hello.\n> > > Tomorrow I will test and send you an answer.\n> > >\n> > > Thank you\n> > > Nejc Galof\n> > >\n> > > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart <\n> > > laurent.pinchart@ideasonboard.com> napisala:\n> > >\n> > >> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> > >> write-only mappings). We can support this as the dmabuf mmap()\n> > >> implementation supports it. This change fixes usage of the V4L2\n> > >> compatibility layer with OpenCV.\n> > >>\n> > >> While at it, attempt to validate the other flags. videobuf2 requires\n> > >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> > >> While unlikly, other flags could get rejected by other kernel layers for\n> > >> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> > >> arises.\n> > >>\n> > >> Signed-off-by: Laurent Pinchart \n> > >> ---\n> > >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> > >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> > >>\n> > >> Nejc, could you please test this patch ? It's slightly different than\n> > >> the one I've shared on the IRC channel. If you don't mind appearing in\n> > >> the git development history, you can reply with\n> > >>\n> > >> Tested-by: Name \n> > >>\n> > >> and I'll add that (as well as a corresponding Reported-by tag) to the\n> > >> commit before pushing it.\n> > >>\n> > >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> > >> b/src/v4l2/v4l2_camera_proxy.cpp\n> > >> index f3470a6d312a..ebe7601a4ba6 100644\n> > >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> > >> void *addr, size_t length,\n> > >>\n> > >> MutexLocker locker(proxyMutex_);\n> > >>\n> > >> - /* \\todo Validate prot and flags properly. */\n> > >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> > >> + /*\n> > >> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> > >> + * PROT_EXEC as it makes no sense.\n> > >> + */\n> > >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n\nLooking through\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/common/videobuf2/videobuf2-core.c#n2260\n\nwhich I expect is the equivalent that this code emulates, I see the\nMAP_SHARED requirement, but there is no reference to PROT_EXEC that I\ncan see.\n\nWhile it may not make sense supporting executable code in a capture\nbuffer, does it add any specific benefit to restricting it here? or does\nit cause the potential to cause an app that mapped with PROT_READ |\nPROT_EXEC that would work with the kernel, but fail with our V4L2 layer?\n(if there was perhaps some legitimate reason to capture into an\nexecutable buffer which I doubt).\n\nMaybe if this restriction really should be added - it should also be\nadded to the kernel too - but aside from that, I think it might help to\nat least print some reasons why we are failing the call explicitly.\n\nIdeally with some debug prints of the cause of the failure:\n\n\nReviewed-by: Kieran Bingham \n\n\n> > >> + errno = EINVAL;\n> > >> + return MAP_FAILED;\n> > >> + }\n> > >> +\n> > >> + /* videobuf2 also requires MAP_SHARED. */\n> > >> + if (!(flags & MAP_SHARED)) {\n> > >> errno = EINVAL;\n> > >> return MAP_FAILED;\n> > >> }\n> > >>\n> > >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AC87ABDCBF\n\tfor ;\n\tSat, 29 Jan 2022 20:43:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3EEA609C7;\n\tSat, 29 Jan 2022 21:43:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 009D6609A9\n\tfor ;\n\tSat, 29 Jan 2022 21:42:58 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9278F291;\n\tSat, 29 Jan 2022 21:42:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Lt9vBbqh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643488978;\n\tbh=Cd4U1tFE1c//s7ztM03O7w7I6RvQLAj793h4GttjB0U=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Lt9vBbqhe1Qb3iAa5UKTeoRKb8mKyJ3WPfKU10afUrjf/Rl5e0Umv2KI9U5C51RsO\n\t/R3b5ueaieDsF9NKCUFeme1uI65J7GqIojdNhrFgXWYG1lodauRMgOZxIc85NAMqEM\n\tw14hzqKe0HTOexu7XxgX71JY4X2s6+RvJq2e4mEo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tNejc Galof ","Date":"Sat, 29 Jan 2022 20:42:56 +0000","Message-ID":"<164348897616.1778844.4614155599174754915@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22074,"web_url":"https://patchwork.libcamera.org/comment/22074/","msgid":"","date":"2022-01-30T01:19:41","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Sat, Jan 29, 2022 at 08:42:56PM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2022-01-28 22:27:47)\n> > Hi Nejc,\n> > \n> > On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> > > Tested-by: Nejc \n> > \n> > I'll update this to\n> > \n> > Tested-by: Nejc Galof \n> > \n> > and add\n> > \n> > Reported-by: Nejc Galof \n> > \n> > Thank you for your first contribution to libcamera :-)\n> > \n> > > V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof napisala:\n> > > \n> > > > Hello.\n> > > > Tomorrow I will test and send you an answer.\n> > > >\n> > > > Thank you\n> > > > Nejc Galof\n> > > >\n> > > > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart napisala:\n> > > >\n> > > >> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> > > >> write-only mappings). We can support this as the dmabuf mmap()\n> > > >> implementation supports it. This change fixes usage of the V4L2\n> > > >> compatibility layer with OpenCV.\n> > > >>\n> > > >> While at it, attempt to validate the other flags. videobuf2 requires\n> > > >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> > > >> While unlikly, other flags could get rejected by other kernel layers for\n> > > >> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> > > >> arises.\n> > > >>\n> > > >> Signed-off-by: Laurent Pinchart \n> > > >> ---\n> > > >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> > > >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> > > >>\n> > > >> Nejc, could you please test this patch ? It's slightly different than\n> > > >> the one I've shared on the IRC channel. If you don't mind appearing in\n> > > >> the git development history, you can reply with\n> > > >>\n> > > >> Tested-by: Name \n> > > >>\n> > > >> and I'll add that (as well as a corresponding Reported-by tag) to the\n> > > >> commit before pushing it.\n> > > >>\n> > > >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> > > >> b/src/v4l2/v4l2_camera_proxy.cpp\n> > > >> index f3470a6d312a..ebe7601a4ba6 100644\n> > > >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> > > >> void *addr, size_t length,\n> > > >>\n> > > >> MutexLocker locker(proxyMutex_);\n> > > >>\n> > > >> - /* \\todo Validate prot and flags properly. */\n> > > >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> > > >> + /*\n> > > >> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> > > >> + * PROT_EXEC as it makes no sense.\n> > > >> + */\n> > > >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> \n> Looking through\n> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/common/videobuf2/videobuf2-core.c#n2260\n> \n> which I expect is the equivalent that this code emulates, I see the\n> MAP_SHARED requirement, but there is no reference to PROT_EXEC that I\n> can see.\n\nCorrect.\n\n> While it may not make sense supporting executable code in a capture\n> buffer, does it add any specific benefit to restricting it here? or does\n> it cause the potential to cause an app that mapped with PROT_READ |\n> PROT_EXEC that would work with the kernel, but fail with our V4L2 layer?\n> (if there was perhaps some legitimate reason to capture into an\n> executable buffer which I doubt).\n> \n> Maybe if this restriction really should be added - it should also be\n> added to the kernel too - but aside from that, I think it might help to\n> at least print some reasons why we are failing the call explicitly.\n\nI'll drop the check for now, as there's no similar check being performed\non the kernel side, even if I fail to find any valid use case.\n\n> Ideally with some debug prints of the cause of the failure:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > > >> + errno = EINVAL;\n> > > >> + return MAP_FAILED;\n> > > >> + }\n> > > >> +\n> > > >> + /* videobuf2 also requires MAP_SHARED. */\n> > > >> + if (!(flags & MAP_SHARED)) {\n> > > >> errno = EINVAL;\n> > > >> return MAP_FAILED;\n> > > >> }\n> > > >>\n> > > >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C1E3ABF415\n\tfor ;\n\tSun, 30 Jan 2022 01:20:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC828609C7;\n\tSun, 30 Jan 2022 02:20:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ADBAD609A9\n\tfor ;\n\tSun, 30 Jan 2022 02:20:03 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 07E48291;\n\tSun, 30 Jan 2022 02:20:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FNM+v+BW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643505603;\n\tbh=WvrwsVPpiXGNimHj09oRTebcrdl8AAokkMCw/BAPOSI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FNM+v+BWwbHd0n4Q25MkLpreH0xxUEvw4Zmg5kAhAHcbWTLBMzsEBiNNza4VPGziG\n\t/cqssBOyNW2xdpV6uYg58GJaYdUwK9Fboaj7QaS2ISJzg7ZWOdZwC/W3fvHLDt2+UD\n\tFfYHHhJpB2lpXbvSoubkvpyTTfPSJvb8b5/1/ziY=","Date":"Sun, 30 Jan 2022 03:19:41 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t\n\t<164348897616.1778844.4614155599174754915@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<164348897616.1778844.4614155599174754915@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, Nejc Galof ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22085,"web_url":"https://patchwork.libcamera.org/comment/22085/","msgid":"","date":"2022-02-01T09:25:17","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":113,"url":"https://patchwork.libcamera.org/api/people/113/","name":"Nejc Galof","email":"galof.nejc@gmail.com"},"content":"Hello.\nWhen can I expect this fix in the master branch?\n\nThank you for the answer.\nNejc Galof\n\nV V ned., 30. jan. 2022 ob 02:20 je oseba Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> napisala:\n\n> Hi Kieran,\n>\n> On Sat, Jan 29, 2022 at 08:42:56PM +0000, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2022-01-28 22:27:47)\n> > > Hi Nejc,\n> > >\n> > > On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> > > > Tested-by: Nejc \n> > >\n> > > I'll update this to\n> > >\n> > > Tested-by: Nejc Galof \n> > >\n> > > and add\n> > >\n> > > Reported-by: Nejc Galof \n> > >\n> > > Thank you for your first contribution to libcamera :-)\n> > >\n> > > > V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof napisala:\n> > > >\n> > > > > Hello.\n> > > > > Tomorrow I will test and send you an answer.\n> > > > >\n> > > > > Thank you\n> > > > > Nejc Galof\n> > > > >\n> > > > > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart\n> napisala:\n> > > > >\n> > > > >> V4L2 is happy to map buffers read-only for capture devices (but\n> rejects\n> > > > >> write-only mappings). We can support this as the dmabuf mmap()\n> > > > >> implementation supports it. This change fixes usage of the V4L2\n> > > > >> compatibility layer with OpenCV.\n> > > > >>\n> > > > >> While at it, attempt to validate the other flags. videobuf2\n> requires\n> > > > >> MAP_SHARED and doesn't check other flags, so mimic the same\n> behaviour.\n> > > > >> While unlikly, other flags could get rejected by other kernel\n> layers for\n> > > > >> V4L2 buffers but not for dmabuf. This can be handled later if the\n> need\n> > > > >> arises.\n> > > > >>\n> > > > >> Signed-off-by: Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com>\n> > > > >> ---\n> > > > >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> > > > >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> > > > >>\n> > > > >> Nejc, could you please test this patch ? It's slightly different\n> than\n> > > > >> the one I've shared on the IRC channel. If you don't mind\n> appearing in\n> > > > >> the git development history, you can reply with\n> > > > >>\n> > > > >> Tested-by: Name \n> > > > >>\n> > > > >> and I'll add that (as well as a corresponding Reported-by tag) to\n> the\n> > > > >> commit before pushing it.\n> > > > >>\n> > > > >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> index f3470a6d312a..ebe7601a4ba6 100644\n> > > > >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile\n> *file,\n> > > > >> void *addr, size_t length,\n> > > > >>\n> > > > >> MutexLocker locker(proxyMutex_);\n> > > > >>\n> > > > >> - /* \\todo Validate prot and flags properly. */\n> > > > >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> > > > >> + /*\n> > > > >> + * Mimic the videobuf2 behaviour, which requires\n> PROT_READ. Reject\n> > > > >> + * PROT_EXEC as it makes no sense.\n> > > > >> + */\n> > > > >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> >\n> > Looking through\n> >\n> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/common/videobuf2/videobuf2-core.c#n2260\n> >\n> > which I expect is the equivalent that this code emulates, I see the\n> > MAP_SHARED requirement, but there is no reference to PROT_EXEC that I\n> > can see.\n>\n> Correct.\n>\n> > While it may not make sense supporting executable code in a capture\n> > buffer, does it add any specific benefit to restricting it here? or does\n> > it cause the potential to cause an app that mapped with PROT_READ |\n> > PROT_EXEC that would work with the kernel, but fail with our V4L2 layer?\n> > (if there was perhaps some legitimate reason to capture into an\n> > executable buffer which I doubt).\n> >\n> > Maybe if this restriction really should be added - it should also be\n> > added to the kernel too - but aside from that, I think it might help to\n> > at least print some reasons why we are failing the call explicitly.\n>\n> I'll drop the check for now, as there's no similar check being performed\n> on the kernel side, even if I fail to find any valid use case.\n>\n> > Ideally with some debug prints of the cause of the failure:\n> >\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > > > >> + errno = EINVAL;\n> > > > >> + return MAP_FAILED;\n> > > > >> + }\n> > > > >> +\n> > > > >> + /* videobuf2 also requires MAP_SHARED. */\n> > > > >> + if (!(flags & MAP_SHARED)) {\n> > > > >> errno = EINVAL;\n> > > > >> return MAP_FAILED;\n> > > > >> }\n> > > > >>\n> > > > >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A62B4BDCBF\n\tfor ;\n\tTue, 1 Feb 2022 09:25:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0773B609DD;\n\tTue, 1 Feb 2022 10:25:31 +0100 (CET)","from mail-io1-xd32.google.com (mail-io1-xd32.google.com\n\t[IPv6:2607:f8b0:4864:20::d32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89D75609AC\n\tfor ;\n\tTue, 1 Feb 2022 10:25:29 +0100 (CET)","by mail-io1-xd32.google.com with SMTP id y84so20445055iof.0\n\tfor ;\n\tTue, 01 Feb 2022 01:25:29 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Wv5b9r/V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=8XPUJxOMO76iZ5ypjuK4gF6qJLY/l9/z8vR2JaRNqIQ=;\n\tb=Wv5b9r/VUEQjJjF032PR/ERn/GDt9ECNRbeCWqGyucAStDkwFNgOoqYVwdnCbqTjjR\n\taSsT0H1AuLjRFQu/Chh09d0LxYxVey4q158jD76tSDjmcXlI1nMj7dCIu+JwjJP7WA8o\n\tGsx1l+APm4+P4q6Q1+sL9qXzoJNZC86jwBJLoD5UA30GkTdP2QqqLGCIOO7QqHmShztb\n\tanf33VGGGCXCoF1gBKaoMac3KCf6jmHuyCvG+ZQl7KwsccexrCZs78xKk8160IPB/eTP\n\tNElZbbnm2YCKJQdqRZX831RXtlonTWch3sNspymDxvN5UJ5QDYjXqCZ37kFkWOiSnayx\n\tysMg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=8XPUJxOMO76iZ5ypjuK4gF6qJLY/l9/z8vR2JaRNqIQ=;\n\tb=vf6pugI5LDjh+OuIWmgrnp+O3OJN0m5xIwpl2WpfiCXH1Q6gS9kdw6YLqnIbaVwrQg\n\tWWS+7R7lvQEB7+4HDrygRBAcT4BP0clAUY5Ww34iNOVurd9vC/4TA6HtIVEg2hGynb5T\n\tSmqTK7d7BcAX9oU6iFGd5G1W+ynVA0Dl3l8zi6jYV1I6YjJjyAjtvInZn2RaS/N3nQzy\n\tB1Q/bpB1PhgVUr3tCxHlTqmtIefe6D/ezbhUslVBg1Gsq5NwV5Uskq8l0busZEoPalSs\n\trrQq0Cckp/uTSxK8UlGRk2NHF8mJHVkWTca3TUY3nH9mKuZUlvfSXLhgDcFQfV0OdSVV\n\tE+Pw==","X-Gm-Message-State":"AOAM531wD5NQr0aDB/Vcw3vM4yHwCyxhzONAcQiOzfn+tVqcY4iD9Fpw\n\tdRz5asyMr0RwMYl/4zXPiP2eSdN0k6FimMddNrewJBe6yMA=","X-Google-Smtp-Source":"ABdhPJzCo9EeJB9xkynqL3lEmp/KXsoKMwztz0YYrqe4vjwieSN/Dia+P1ILPkxoSYakZAK+GoSUVLeTu6Fv/ON9Dn0=","X-Received":"by 2002:a6b:7316:: with SMTP id\n\te22mr13272848ioh.125.1643707528199; \n\tTue, 01 Feb 2022 01:25:28 -0800 (PST)","MIME-Version":"1.0","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t\n\t<164348897616.1778844.4614155599174754915@Monstersaurus>\n\t","In-Reply-To":"","From":"Nejc Galof ","Date":"Tue, 1 Feb 2022 10:25:17 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000055435b05d6f17b0c\"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22098,"web_url":"https://patchwork.libcamera.org/comment/22098/","msgid":"<20220202044508.GA174255@pyrite.rasen.tech>","date":"2022-02-02T04:45:08","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Sun, Jan 30, 2022 at 03:19:41AM +0200, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Sat, Jan 29, 2022 at 08:42:56PM +0000, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2022-01-28 22:27:47)\n> > > Hi Nejc,\n> > > \n> > > On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> > > > Tested-by: Nejc \n> > > \n> > > I'll update this to\n> > > \n> > > Tested-by: Nejc Galof \n> > > \n> > > and add\n> > > \n> > > Reported-by: Nejc Galof \n> > > \n> > > Thank you for your first contribution to libcamera :-)\n> > > \n> > > > V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof napisala:\n> > > > \n> > > > > Hello.\n> > > > > Tomorrow I will test and send you an answer.\n> > > > >\n> > > > > Thank you\n> > > > > Nejc Galof\n> > > > >\n> > > > > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart napisala:\n> > > > >\n> > > > >> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> > > > >> write-only mappings). We can support this as the dmabuf mmap()\n> > > > >> implementation supports it. This change fixes usage of the V4L2\n> > > > >> compatibility layer with OpenCV.\n> > > > >>\n> > > > >> While at it, attempt to validate the other flags. videobuf2 requires\n> > > > >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> > > > >> While unlikly, other flags could get rejected by other kernel layers for\n> > > > >> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> > > > >> arises.\n> > > > >>\n> > > > >> Signed-off-by: Laurent Pinchart \n> > > > >> ---\n> > > > >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> > > > >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> > > > >>\n> > > > >> Nejc, could you please test this patch ? It's slightly different than\n> > > > >> the one I've shared on the IRC channel. If you don't mind appearing in\n> > > > >> the git development history, you can reply with\n> > > > >>\n> > > > >> Tested-by: Name \n> > > > >>\n> > > > >> and I'll add that (as well as a corresponding Reported-by tag) to the\n> > > > >> commit before pushing it.\n> > > > >>\n> > > > >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> index f3470a6d312a..ebe7601a4ba6 100644\n> > > > >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> > > > >> void *addr, size_t length,\n> > > > >>\n> > > > >> MutexLocker locker(proxyMutex_);\n> > > > >>\n> > > > >> - /* \\todo Validate prot and flags properly. */\n> > > > >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> > > > >> + /*\n> > > > >> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> > > > >> + * PROT_EXEC as it makes no sense.\n> > > > >> + */\n> > > > >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> > \n> > Looking through\n> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/common/videobuf2/videobuf2-core.c#n2260\n> > \n> > which I expect is the equivalent that this code emulates, I see the\n> > MAP_SHARED requirement, but there is no reference to PROT_EXEC that I\n> > can see.\n> \n> Correct.\n> \n> > While it may not make sense supporting executable code in a capture\n> > buffer, does it add any specific benefit to restricting it here? or does\n> > it cause the potential to cause an app that mapped with PROT_READ |\n> > PROT_EXEC that would work with the kernel, but fail with our V4L2 layer?\n> > (if there was perhaps some legitimate reason to capture into an\n> > executable buffer which I doubt).\n> > \n> > Maybe if this restriction really should be added - it should also be\n> > added to the kernel too - but aside from that, I think it might help to\n> > at least print some reasons why we are failing the call explicitly.\n> \n> I'll drop the check for now, as there's no similar check being performed\n> on the kernel side, even if I fail to find any valid use case.\n\nWith the check dropped,\n\nReviewed-by: Paul Elder \n\n> \n> > Ideally with some debug prints of the cause of the failure:\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > > >> + errno = EINVAL;\n> > > > >> + return MAP_FAILED;\n> > > > >> + }\n> > > > >> +\n> > > > >> + /* videobuf2 also requires MAP_SHARED. */\n> > > > >> + if (!(flags & MAP_SHARED)) {\n> > > > >> errno = EINVAL;\n> > > > >> return MAP_FAILED;\n> > > > >> }\n> > > > >>\n> > > > >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 00871BF415\n\tfor ;\n\tWed, 2 Feb 2022 04:45:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24A4D609DF;\n\tWed, 2 Feb 2022 05:45:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CA696020A\n\tfor ;\n\tWed, 2 Feb 2022 05:45:18 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4900C2F3;\n\tWed, 2 Feb 2022 05:45:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NC6KJZv9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643777118;\n\tbh=xqXpJVdWei0Lg+QAElP4zz11h1ZUjIPoRvMxlKZiOS4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NC6KJZv9bBMi/NT5Tz0fkwHoldHsp4D857/YUN+pVy9ahN0BDV6F5EQhO6yWH3MVS\n\t1FmMlD7MCWeec4hCbAUjlV/z+ddTrMC3JlbN0ryLOfXQyRpS2dcJOwmBCAG0Gdjhc4\n\tASDS8NiMv8qlMeOLXs94ac0K48htxDMYhHVe39Tg=","Date":"Wed, 2 Feb 2022 13:45:08 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20220202044508.GA174255@pyrite.rasen.tech>","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t\n\t<164348897616.1778844.4614155599174754915@Monstersaurus>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, Nejc Galof ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22099,"web_url":"https://patchwork.libcamera.org/comment/22099/","msgid":"","date":"2022-02-02T06:17:33","subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nejc,\n\nOn Tue, Feb 01, 2022 at 10:25:17AM +0100, Nejc Galof wrote:\n> Hello.\n> When can I expect this fix in the master branch?\n\nI was waiting for Paul to give it a look. It's done now, and I've pushed\nthe fix.\n\n> V V ned., 30. jan. 2022 ob 02:20 je oseba Laurent Pinchart napisala:\n> > On Sat, Jan 29, 2022 at 08:42:56PM +0000, Kieran Bingham wrote:\n> > > Quoting Laurent Pinchart (2022-01-28 22:27:47)\n> > > > On Fri, Jan 28, 2022 at 11:18:01PM +0100, Nejc Galof wrote:\n> > > > > Tested-by: Nejc \n> > > >\n> > > > I'll update this to\n> > > >\n> > > > Tested-by: Nejc Galof \n> > > >\n> > > > and add\n> > > >\n> > > > Reported-by: Nejc Galof \n> > > >\n> > > > Thank you for your first contribution to libcamera :-)\n> > > >\n> > > > > V V pet., 28. jan. 2022 ob 23:02 je oseba Nejc Galof napisala:\n> > > > >\n> > > > > > Hello.\n> > > > > > Tomorrow I will test and send you an answer.\n> > > > > >\n> > > > > > Thank you\n> > > > > > Nejc Galof\n> > > > > >\n> > > > > > V V pet., 28. jan. 2022 ob 23:00 je oseba Laurent Pinchart napisala:\n> > > > > >\n> > > > > >> V4L2 is happy to map buffers read-only for capture devices (but rejects\n> > > > > >> write-only mappings). We can support this as the dmabuf mmap()\n> > > > > >> implementation supports it. This change fixes usage of the V4L2\n> > > > > >> compatibility layer with OpenCV.\n> > > > > >>\n> > > > > >> While at it, attempt to validate the other flags. videobuf2 requires\n> > > > > >> MAP_SHARED and doesn't check other flags, so mimic the same behaviour.\n> > > > > >> While unlikly, other flags could get rejected by other kernel layers for\n> > > > > >> V4L2 buffers but not for dmabuf. This can be handled later if the need\n> > > > > >> arises.\n> > > > > >>\n> > > > > >> Signed-off-by: Laurent Pinchart \n> > > > > >> ---\n> > > > > >> src/v4l2/v4l2_camera_proxy.cpp | 13 +++++++++++--\n> > > > > >> 1 file changed, 11 insertions(+), 2 deletions(-)\n> > > > > >>\n> > > > > >> Nejc, could you please test this patch ? It's slightly different than\n> > > > > >> the one I've shared on the IRC channel. If you don't mind appearing in\n> > > > > >> the git development history, you can reply with\n> > > > > >>\n> > > > > >> Tested-by: Name \n> > > > > >>\n> > > > > >> and I'll add that (as well as a corresponding Reported-by tag) to the\n> > > > > >> commit before pushing it.\n> > > > > >>\n> > > > > >> diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > >> b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > >> index f3470a6d312a..ebe7601a4ba6 100644\n> > > > > >> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > >> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > >> @@ -104,8 +104,17 @@ void *V4L2CameraProxy::mmap(V4L2CameraFile *file,\n> > > > > >> void *addr, size_t length,\n> > > > > >>\n> > > > > >> MutexLocker locker(proxyMutex_);\n> > > > > >>\n> > > > > >> - /* \\todo Validate prot and flags properly. */\n> > > > > >> - if (prot != (PROT_READ | PROT_WRITE)) {\n> > > > > >> + /*\n> > > > > >> + * Mimic the videobuf2 behaviour, which requires PROT_READ. Reject\n> > > > > >> + * PROT_EXEC as it makes no sense.\n> > > > > >> + */\n> > > > > >> + if (!(prot & PROT_READ) || prot & PROT_EXEC) {\n> > >\n> > > Looking through\n> > >\n> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/common/videobuf2/videobuf2-core.c#n2260\n> > >\n> > > which I expect is the equivalent that this code emulates, I see the\n> > > MAP_SHARED requirement, but there is no reference to PROT_EXEC that I\n> > > can see.\n> >\n> > Correct.\n> >\n> > > While it may not make sense supporting executable code in a capture\n> > > buffer, does it add any specific benefit to restricting it here? or does\n> > > it cause the potential to cause an app that mapped with PROT_READ |\n> > > PROT_EXEC that would work with the kernel, but fail with our V4L2 layer?\n> > > (if there was perhaps some legitimate reason to capture into an\n> > > executable buffer which I doubt).\n> > >\n> > > Maybe if this restriction really should be added - it should also be\n> > > added to the kernel too - but aside from that, I think it might help to\n> > > at least print some reasons why we are failing the call explicitly.\n> >\n> > I'll drop the check for now, as there's no similar check being performed\n> > on the kernel side, even if I fail to find any valid use case.\n> >\n> > > Ideally with some debug prints of the cause of the failure:\n> > >\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > > > > >> + errno = EINVAL;\n> > > > > >> + return MAP_FAILED;\n> > > > > >> + }\n> > > > > >> +\n> > > > > >> + /* videobuf2 also requires MAP_SHARED. */\n> > > > > >> + if (!(flags & MAP_SHARED)) {\n> > > > > >> errno = EINVAL;\n> > > > > >> return MAP_FAILED;\n> > > > > >> }\n> > > > > >>\n> > > > > >> base-commit: 7ea52d2b586144fdc033a3ffc1c4a4bbb99b5440","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DACBEBDCBF\n\tfor ;\n\tWed, 2 Feb 2022 06:17:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 442BF609B9;\n\tWed, 2 Feb 2022 07:17:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1ADF6020A\n\tfor ;\n\tWed, 2 Feb 2022 07:17:56 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5DB312F3;\n\tWed, 2 Feb 2022 07:17:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"n1RqZbww\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643782676;\n\tbh=fgJJboRb7U0wYE9Puc0GvT1igVDXGIgQdlH2DPztvG0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n1RqZbww057sV72XViZEoRGdCsKOChB+rWfPYt/DsHYmJL2GEj5RjiFJ6RM72ALT+\n\ttdFJq4VCfMu2G6TKPEvcYAbsXgPDKQQ9mOvKF6MpECsQfRLbjXsY126X1GcKjBQzCQ\n\tBzMZQzBu5kHXNsHZQ2F07Vb33TCKJ0UKY57QzsOc=","Date":"Wed, 2 Feb 2022 08:17:33 +0200","From":"Laurent Pinchart ","To":"Nejc Galof ","Message-ID":"","References":"<20220128220031.3467-1-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t\n\t<164348897616.1778844.4614155599174754915@Monstersaurus>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: Accept read-only buffers\n\tmappings and require MAP_SHARED","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]