[{"id":5230,"web_url":"https://patchwork.libcamera.org/comment/5230/","msgid":"<20200617010607.GR913@pendragon.ideasonboard.com>","date":"2020-06-17T01:06:07","subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Tue, Jun 16, 2020 at 10:12:31PM +0900, Paul Elder wrote:\n> The ioctl handlers currently don't check if arg is null, so if it ever\n> is, it will cause a segfault. Check that arg is null and return -EFAULT\n> in all vidioc ioctl handlers.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\nCan't you check that in V4L2CameraProxy::ioctl() ?\n\n> ---\n>  src/v4l2/v4l2_camera_proxy.cpp | 33 +++++++++++++++++++++++++++++++--\n>  1 file changed, 31 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 594dd13..5b74b53 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \t*arg = capabilities_;\n>  \n>  \treturn 0;\n> @@ -247,6 +250,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enum_fmt fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n>  \n>  \tif (!validateBufferType(arg->type) ||\n>  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> @@ -264,6 +269,8 @@ int V4L2CameraProxy::vidioc_g_fmt(int fd, struct v4l2_format *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_fmt fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n>  \n>  \tif (!validateBufferType(arg->type))\n>  \t\treturn -EINVAL;\n> @@ -303,6 +310,9 @@ int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_fmt fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n> @@ -334,6 +344,9 @@ int V4L2CameraProxy::vidioc_try_fmt(int fd, struct v4l2_format *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_try_fmt fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tif (!validateBufferType(arg->type))\n>  \t\treturn -EINVAL;\n>  \n> @@ -361,6 +374,8 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_reqbufs fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n>  \n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n> @@ -444,6 +459,9 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querybuf fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n> @@ -461,8 +479,10 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n>  \n>  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)\n>  {\n> -\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> -\t\t\t       << arg->index << \" fd = \" << fd;\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf fd = \" << fd;\n> +\n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n>  \n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n> @@ -487,6 +507,9 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n> @@ -522,6 +545,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n> @@ -538,6 +564,9 @@ int V4L2CameraProxy::vidioc_streamoff(int fd, int *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamoff fd = \" << fd;\n>  \n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n>  \tint ret = lock(fd);\n>  \tif (ret < 0)\n>  \t\treturn ret;","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 64B80603D8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 03:06:30 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E9F20F9;\n\tWed, 17 Jun 2020 03:06:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sJYY86jS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592355990;\n\tbh=6Qf+pnLCuP2m6yReRBAa9zGmXniGOf1pVK9nkQMN010=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sJYY86jSghGdnuEj7bftTYpv3TcVDtE0MV/ikb36GSj7pBjK9VJbaLRm92v1oLwcq\n\tP617y5PUNBnNnMDjTGZYQ3YsmDYcbsmda171NbuG2Dqu8XT0JpXBa+oymi3rlp42SZ\n\tia0p7L9Mb4mCAD8PkjWrE0fg3eZT4kdxrzfD/JD0=","Date":"Wed, 17 Jun 2020 04:06:07 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200617010607.GR913@pendragon.ideasonboard.com>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200616131244.70308-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 01:06:30 -0000"}},{"id":5234,"web_url":"https://patchwork.libcamera.org/comment/5234/","msgid":"<20200617093031.GA2440@jade.amanokami.net>","date":"2020-06-17T09:30:31","subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Jun 17, 2020 at 04:06:07AM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Tue, Jun 16, 2020 at 10:12:31PM +0900, Paul Elder wrote:\n> > The ioctl handlers currently don't check if arg is null, so if it ever\n> > is, it will cause a segfault. Check that arg is null and return -EFAULT\n> > in all vidioc ioctl handlers.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> Can't you check that in V4L2CameraProxy::ioctl() ?\n\nWe can't do that. v4l2-compliance requires that 1) querycap shold not\nreturn EFAULT when arg == NULL, and 2) invalid ioctl should return\nENOTTY. Because of the second point, we would still have to check at\nevery ioctl.\n\n\nPaul\n\n\n> > ---\n> >  src/v4l2/v4l2_camera_proxy.cpp | 33 +++++++++++++++++++++++++++++++--\n> >  1 file changed, 31 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 594dd13..5b74b53 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \t*arg = capabilities_;\n> >  \n> >  \treturn 0;\n> > @@ -247,6 +250,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enum_fmt fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >  \n> >  \tif (!validateBufferType(arg->type) ||\n> >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> > @@ -264,6 +269,8 @@ int V4L2CameraProxy::vidioc_g_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_fmt fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >  \n> >  \tif (!validateBufferType(arg->type))\n> >  \t\treturn -EINVAL;\n> > @@ -303,6 +310,9 @@ int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_fmt fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -334,6 +344,9 @@ int V4L2CameraProxy::vidioc_try_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_try_fmt fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tif (!validateBufferType(arg->type))\n> >  \t\treturn -EINVAL;\n> >  \n> > @@ -361,6 +374,8 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_reqbufs fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >  \n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> > @@ -444,6 +459,9 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querybuf fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -461,8 +479,10 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> >  \n> >  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> > -\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > -\t\t\t       << arg->index << \" fd = \" << fd;\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf fd = \" << fd;\n> > +\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >  \n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> > @@ -487,6 +507,9 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -522,6 +545,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -538,6 +564,9 @@ int V4L2CameraProxy::vidioc_streamoff(int fd, int *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamoff fd = \" << fd;\n> >  \n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<paul.elder@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0653603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 11:30:40 +0200 (CEST)","from jade.amanokami.net (unknown\n\t[IPv6:2400:4051:61:600:4409:e6df:9984:3512])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA54A2BD;\n\tWed, 17 Jun 2020 11:30:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MVv7Tcsl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592386240;\n\tbh=+f2DDW/fjEPN/hR4/Bl3a6KKqY2ujlfHcgPX2pUDd2g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MVv7TcsllOSgHuhQqfd/1CefD0cyNkKHgu9Xgvut1OpDqEhcNpV7Yh4xBSbOtGJHp\n\taovn3iouu+ufAKGvidT9ZKswt2UGpyt+njn6UqzjgHDWm1VkdwmlwSUIWY+LM7zbeA\n\trhq5pDKN+A6dN6RKQqzNfKCsvon/9KvD61gM0Dvo=","Date":"Wed, 17 Jun 2020 18:30:31 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200617093031.GA2440@jade.amanokami.net>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-3-paul.elder@ideasonboard.com>\n\t<20200617010607.GR913@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200617010607.GR913@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 09:30:41 -0000"}},{"id":5237,"web_url":"https://patchwork.libcamera.org/comment/5237/","msgid":"<20200617122043.my44nr3bgiqh4kup@uno.localdomain>","date":"2020-06-17T12:20:43","subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul, Laurent\n\nOn Wed, Jun 17, 2020 at 04:06:07AM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n>\n> Thank you for the patch.\n>\n> On Tue, Jun 16, 2020 at 10:12:31PM +0900, Paul Elder wrote:\n> > The ioctl handlers currently don't check if arg is null, so if it ever\n> > is, it will cause a segfault. Check that arg is null and return -EFAULT\n> > in all vidioc ioctl handlers.\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n> Can't you check that in V4L2CameraProxy::ioctl() ?\n>\n\nAnd maybe use !arg  for the check ?\n\nThanks\n  j\n\n> > ---\n> >  src/v4l2/v4l2_camera_proxy.cpp | 33 +++++++++++++++++++++++++++++++--\n> >  1 file changed, 31 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 594dd13..5b74b53 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \t*arg = capabilities_;\n> >\n> >  \treturn 0;\n> > @@ -247,6 +250,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enum_fmt fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >\n> >  \tif (!validateBufferType(arg->type) ||\n> >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> > @@ -264,6 +269,8 @@ int V4L2CameraProxy::vidioc_g_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_fmt fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >\n> >  \tif (!validateBufferType(arg->type))\n> >  \t\treturn -EINVAL;\n> > @@ -303,6 +310,9 @@ int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_fmt fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -334,6 +344,9 @@ int V4L2CameraProxy::vidioc_try_fmt(int fd, struct v4l2_format *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_try_fmt fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tif (!validateBufferType(arg->type))\n> >  \t\treturn -EINVAL;\n> >\n> > @@ -361,6 +374,8 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_reqbufs fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> > @@ -444,6 +459,9 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querybuf fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -461,8 +479,10 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> >\n> >  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> > -\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > -\t\t\t       << arg->index << \" fd = \" << fd;\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf fd = \" << fd;\n> > +\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> >\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> > @@ -487,6 +507,9 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -522,6 +545,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> > @@ -538,6 +564,9 @@ int V4L2CameraProxy::vidioc_streamoff(int fd, int *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamoff fd = \" << fd;\n> >\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> >  \tint ret = lock(fd);\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C884F603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 14:17:18 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 292D0C000B;\n\tWed, 17 Jun 2020 12:17:17 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 17 Jun 2020 14:20:43 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617122043.my44nr3bgiqh4kup@uno.localdomain>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-3-paul.elder@ideasonboard.com>\n\t<20200617010607.GR913@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200617010607.GR913@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 12:17:18 -0000"}},{"id":5238,"web_url":"https://patchwork.libcamera.org/comment/5238/","msgid":"<20200617123209.uauugi4ios5qnf35@uno.localdomain>","date":"2020-06-17T12:32:09","subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Wed, Jun 17, 2020 at 06:30:31PM +0900, Paul Elder wrote:\n> On Wed, Jun 17, 2020 at 04:06:07AM +0300, Laurent Pinchart wrote:\n> > Hi Paul,\n> >\n> > Thank you for the patch.\n> >\n> > On Tue, Jun 16, 2020 at 10:12:31PM +0900, Paul Elder wrote:\n> > > The ioctl handlers currently don't check if arg is null, so if it ever\n> > > is, it will cause a segfault. Check that arg is null and return -EFAULT\n> > > in all vidioc ioctl handlers.\n> > >\n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> >\n> > Can't you check that in V4L2CameraProxy::ioctl() ?\n>\n> We can't do that. v4l2-compliance requires that 1) querycap shold not\n> return EFAULT when arg == NULL, and 2) invalid ioctl should return\n\n@@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n {\n        LOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n\n+       if (arg == nullptr)\n+               return -EFAULT;\n+\n\nDoesn't this return EFAUL on querycap with !arg ?\n\n> ENOTTY. Because of the second point, we would still have to check at\n> every ioctl.\n\nYou could work it around with a\nbool is_ioctl_valid(unsigned long request)\n{\n        switch (request) {\n        case VIDIOC_QUERYCAP:\n        case VIDIOC_G_FMT:\n        ....\n                return true;\n        default:\n                return false;\n        }\n}\n\nIf you happen to have to check for an ioctl to be supported in\nmultiple places, otherwise I'm not sure it's worth it.\n\n>\n>\n> Paul\n>\n>\n> > > ---\n> > >  src/v4l2/v4l2_camera_proxy.cpp | 33 +++++++++++++++++++++++++++++++--\n> > >  1 file changed, 31 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index 594dd13..5b74b53 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \t*arg = capabilities_;\n> > >\n> > >  \treturn 0;\n> > > @@ -247,6 +250,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enum_fmt fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >\n> > >  \tif (!validateBufferType(arg->type) ||\n> > >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> > > @@ -264,6 +269,8 @@ int V4L2CameraProxy::vidioc_g_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_fmt fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >\n> > >  \tif (!validateBufferType(arg->type))\n> > >  \t\treturn -EINVAL;\n> > > @@ -303,6 +310,9 @@ int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_fmt fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -334,6 +344,9 @@ int V4L2CameraProxy::vidioc_try_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_try_fmt fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tif (!validateBufferType(arg->type))\n> > >  \t\treturn -EINVAL;\n> > >\n> > > @@ -361,6 +374,8 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_reqbufs fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > > @@ -444,6 +459,9 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querybuf fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -461,8 +479,10 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> > >\n> > >  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > > -\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > > -\t\t\t       << arg->index << \" fd = \" << fd;\n> > > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf fd = \" << fd;\n> > > +\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > > @@ -487,6 +507,9 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -522,6 +545,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -538,6 +564,9 @@ int V4L2CameraProxy::vidioc_streamoff(int fd, int *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamoff fd = \" << fd;\n> > >\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0663F603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 14:28:45 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 4463B1C0004;\n\tWed, 17 Jun 2020 12:28:43 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 17 Jun 2020 14:32:09 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617123209.uauugi4ios5qnf35@uno.localdomain>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-3-paul.elder@ideasonboard.com>\n\t<20200617010607.GR913@pendragon.ideasonboard.com>\n\t<20200617093031.GA2440@jade.amanokami.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200617093031.GA2440@jade.amanokami.net>","Subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 12:28:45 -0000"}},{"id":5247,"web_url":"https://patchwork.libcamera.org/comment/5247/","msgid":"<20200617152154.GF5838@pendragon.ideasonboard.com>","date":"2020-06-17T15:21:54","subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Jun 17, 2020 at 06:30:31PM +0900, Paul Elder wrote:\n> On Wed, Jun 17, 2020 at 04:06:07AM +0300, Laurent Pinchart wrote:\n> > On Tue, Jun 16, 2020 at 10:12:31PM +0900, Paul Elder wrote:\n> > > The ioctl handlers currently don't check if arg is null, so if it ever\n> > > is, it will cause a segfault. Check that arg is null and return -EFAULT\n> > > in all vidioc ioctl handlers.\n> > > \n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > Can't you check that in V4L2CameraProxy::ioctl() ?\n> \n> We can't do that. v4l2-compliance requires that 1) querycap shold not\n> return EFAULT when arg == NULL,\n\nReally ? v4l2-compliance has\n\n\tfail_on_test(doioctl(node, VIDIOC_QUERYCAP, NULL) != EFAULT);\n\nDoesn't it mean it expects EFAULT when arg == NULL ?\n\n> and 2) invalid ioctl should return\n> ENOTTY. Because of the second point, we would still have to check at\n> every ioctl.\n\nIndeed, that's an annoying one :-( We could have a static const std::set\nof supported ioctls (or a switch/case as Jacopo proposed), and use that\nfor the initial checks. That would require keeping the set in sync with\nthe switch/case that dispatches the ioctls, but that would be less\nerror-prone than requiring a manual null check in every handler.\n\nstd::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n...\n};\n\n...\n\n\tif (supportedIoctls_.find(request) == supportedIoctls_.end())\n\t\treturn -ENOTTY;\n\n\tif (!arg)\n\t\treturn -EFAULT;\n\nNow that I've written this, I've looked at the kernel code and at\nv4l2-compliance. The latter has\n\nstatic int testInvalidIoctls(struct node *node, char type)\n{\n\tunsigned ioc = _IOC(_IOC_NONE, type, 0xff, 0);\n\tunsigned char buf[0x4000] = {};\n\n\tfail_on_test(doioctl(node, ioc, NULL) != ENOTTY);\n\tioc = _IOC(_IOC_NONE, type, 0, 0x3fff);\n\tfail_on_test(doioctl(node, ioc, NULL) != ENOTTY);\n\tioc = _IOC(_IOC_READ, type, 0, 0x3fff);\n\tfail_on_test(doioctl(node, ioc, buf) != ENOTTY);\n\tfail_on_test(check_0(buf, sizeof(buf)));\n\tioc = _IOC(_IOC_WRITE, type, 0, 0x3fff);\n\tfail_on_test(doioctl(node, ioc, buf) != ENOTTY);\n\tfail_on_test(check_0(buf, sizeof(buf)));\n\tioc = _IOC(_IOC_READ | _IOC_WRITE, type, 0, 0x3fff);\n\tfail_on_test(doioctl(node, ioc, buf) != ENOTTY);\n\tfail_on_test(check_0(buf, sizeof(buf)));\n\treturn 0;\n}\n\nNote how the _IOC_WRITE ioctls are not tested with a NULL argument.\nThose would return -EFAULT. How about the following code ?\n\n\tif (!arg && _IOC_DIR(request) & _IOC_WRITE)\n\t\treturn -EFAULT;\n\n\tif (supportedIoctls_.find(request) == supportedIoctls_.end())\n\t\treturn -ENOTTY;\n\n\tif (!arg && _IOC_DIR(request) & _IOC_READ)\n\t\treturn -EFAULT;\n\nThe only ioctl that has neither _IOC_READ nor _IOC_WRITE set is\nVIDIOC_LOG_STATUS. We don't support it, so the last check could\ntechnically be\n\n\tif (!arg)\n\t\treturn -EFAULT;\n\nUp to you.\n\n> > > ---\n> > >  src/v4l2/v4l2_camera_proxy.cpp | 33 +++++++++++++++++++++++++++++++--\n> > >  1 file changed, 31 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index 594dd13..5b74b53 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -238,6 +238,9 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querycap\";\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \t*arg = capabilities_;\n> > >  \n> > >  \treturn 0;\n> > > @@ -247,6 +250,8 @@ int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enum_fmt fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >  \n> > >  \tif (!validateBufferType(arg->type) ||\n> > >  \t    arg->index >= streamConfig_.formats().pixelformats().size())\n> > > @@ -264,6 +269,8 @@ int V4L2CameraProxy::vidioc_g_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_fmt fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >  \n> > >  \tif (!validateBufferType(arg->type))\n> > >  \t\treturn -EINVAL;\n> > > @@ -303,6 +310,9 @@ int V4L2CameraProxy::vidioc_s_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_fmt fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -334,6 +344,9 @@ int V4L2CameraProxy::vidioc_try_fmt(int fd, struct v4l2_format *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_try_fmt fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tif (!validateBufferType(arg->type))\n> > >  \t\treturn -EINVAL;\n> > >  \n> > > @@ -361,6 +374,8 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_reqbufs fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >  \n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > > @@ -444,6 +459,9 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_querybuf fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -461,8 +479,10 @@ int V4L2CameraProxy::vidioc_querybuf(int fd, struct v4l2_buffer *arg)\n> > >  \n> > >  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > > -\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > > -\t\t\t       << arg->index << \" fd = \" << fd;\n> > > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf fd = \" << fd;\n> > > +\n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > >  \n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > > @@ -487,6 +507,9 @@ int V4L2CameraProxy::vidioc_dqbuf(int fd, struct v4l2_buffer *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_dqbuf fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -522,6 +545,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > > @@ -538,6 +564,9 @@ int V4L2CameraProxy::vidioc_streamoff(int fd, int *arg)\n> > >  {\n> > >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamoff fd = \" << fd;\n> > >  \n> > > +\tif (arg == nullptr)\n> > > +\t\treturn -EFAULT;\n> > > +\n> > >  \tint ret = lock(fd);\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4BD07603BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 17 Jun 2020 17:22:18 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B7E3BF9;\n\tWed, 17 Jun 2020 17:22:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vVSbLVU7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592407337;\n\tbh=l29o0Kd/jUAZWid3nIu3vnu2dkm9LOEJm6lePVlCSWk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vVSbLVU7fVnyMx0yuHV7JPblk2MM+RFtjeS4jUyrK/oT8ImmSfbJVt0935pOahJCt\n\tVnumNPiFv4TgMo7NpimnxtquiPDoeRKEIz2nikLzkr1+aCAq19DlHdq8b9JY9ABGY1\n\t/SGVbF1bQCF07aIvdKUU1r8JjMIEmevBYlnU8qts=","Date":"Wed, 17 Jun 2020 18:21:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200617152154.GF5838@pendragon.ideasonboard.com>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-3-paul.elder@ideasonboard.com>\n\t<20200617010607.GR913@pendragon.ideasonboard.com>\n\t<20200617093031.GA2440@jade.amanokami.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200617093031.GA2440@jade.amanokami.net>","Subject":"Re: [libcamera-devel] [PATCH 02/15] v4l2: v4l2_camera_proxy: Check\n\tfor null arg values in ioctl handlers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 17 Jun 2020 15:22:18 -0000"}}]