[{"id":30013,"web_url":"https://patchwork.libcamera.org/comment/30013/","msgid":"<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","date":"2024-06-17T22:37:53","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> `__open64_2()` and `__openat64_2()` should redirect\n> to `open64()` and `openat64()`, respectively, otherwise\n> the `O_LARGEFILE` flag will not be applied.\n> \n> Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n\nCI is all green here.\nhttps://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630 \n\nBut commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\nstates:\n\n    Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n    mmap symbols that we export to not be the 64-bit versions, our dlsym to\n    get the original open and mmap calls will not automatically be converted\n    to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n    versions of open and mmap, we should be using the 64-bit version to\n    service both. Fetch the 64-bit versions of openat and mmap directly.\n\nis that why these were the non-64 bit variants?\n\nThis feels like it's \"too obvious\" ... I wonder what we missed ...\n\nPaul? Any insights? (It was ... a long time ago that you wrote that\npatch)\n\n\n\n> ---\n>  src/v4l2/v4l2_compat.cpp | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> index 8e2b7e92..8a44403e 100644\n> --- a/src/v4l2/v4l2_compat.cpp\n> +++ b/src/v4l2/v4l2_compat.cpp\n> @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n>  \n>  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n>  {\n> -       return open(path, oflag);\n> +       return open64(path, oflag);\n>  }\n>  #endif\n>  \n> @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n>  \n>  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n>  {\n> -       return openat(dirfd, path, oflag);\n> +       return openat64(dirfd, path, oflag);\n>  }\n>  #endif\n>  \n> -- \n> 2.45.2\n> \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 F0D14BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jun 2024 22:37:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BE0F96548B;\n\tTue, 18 Jun 2024 00:37:57 +0200 (CEST)","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 82C8261A20\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jun 2024 00:37:55 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 23F29289;\n\tTue, 18 Jun 2024 00:37:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dociN7u9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718663858;\n\tbh=2Q0i99BuZBY4R/R5Kom5x6uFOG3dPvElr8Mvh8parr4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=dociN7u9OK9kBIeKx/nmYbuRHkJLW2V02Aq1OSly/+ao9p3N+icNMExu0936DApoh\n\tXv2ZH8ES7NgQKotSOrSKxMcOCm0G7IH9ENoH3/4HGk2eAlJsaTL371yNrqbPeCF9bC\n\tZrfa+9blhRNuREjttoqoVa6ktKruGNWt7MKhrfHw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240617214343.2302454-1-pobrn@protonmail.com>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 17 Jun 2024 23:37:53 +0100","Message-ID":"<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30017,"web_url":"https://patchwork.libcamera.org/comment/30017/","msgid":"<20240617232735.GG17726@pendragon.ideasonboard.com>","date":"2024-06-17T23:27:35","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 17, 2024 at 11:37:53PM +0100, Kieran Bingham wrote:\n> Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> > `__open64_2()` and `__openat64_2()` should redirect\n> > to `open64()` and `openat64()`, respectively, otherwise\n> > the `O_LARGEFILE` flag will not be applied.\n> > \n> > Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> \n> CI is all green here.\n> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630 \n> \n> But commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> states:\n> \n>     Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n>     mmap symbols that we export to not be the 64-bit versions, our dlsym to\n>     get the original open and mmap calls will not automatically be converted\n>     to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n>     versions of open and mmap, we should be using the 64-bit version to\n>     service both. Fetch the 64-bit versions of openat and mmap directly.\n> \n> is that why these were the non-64 bit variants?\n\nWith _FORTIFY_SOURCE, we end up servicing both the 32-bit and 64-bit\nversions with the 32-bit wrappers, which then call openat64() that we\ndlsym()ed and stored in V4L2CompatManager::openat. That part is fine,\nbut going through the 32-bit wrappers mean we don't pass O_LARGEFILE to\nopenat64() when called from __open64_2() or __openat64_2(). That's\nwrong, as it will prevent, on 32-bit platforms, file sizes larger than\n4GB.\n\n> This feels like it's \"too obvious\" ... I wonder what we missed ...\n> \n> Paul? Any insights? (It was ... a long time ago that you wrote that\n> patch)\n\nI think the patch is right, it seems to have been an oversight.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > ---\n> >  src/v4l2/v4l2_compat.cpp | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> > index 8e2b7e92..8a44403e 100644\n> > --- a/src/v4l2/v4l2_compat.cpp\n> > +++ b/src/v4l2/v4l2_compat.cpp\n> > @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n> >  \n> >  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n> >  {\n> > -       return open(path, oflag);\n> > +       return open64(path, oflag);\n> >  }\n> >  #endif\n> >  \n> > @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n> >  \n> >  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n> >  {\n> > -       return openat(dirfd, path, oflag);\n> > +       return openat64(dirfd, path, oflag);\n> >  }\n> >  #endif\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A6A75C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jun 2024 23:27:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C96D16548B;\n\tTue, 18 Jun 2024 01:27:58 +0200 (CEST)","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 250B761A20\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jun 2024 01:27:57 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ADFBB289;\n\tTue, 18 Jun 2024 01:27:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oNxok7oF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718666859;\n\tbh=1/m4E/UMycnAXj+r3fciyF+ph0V7aeaT3kPB4Einp18=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oNxok7oFOfS58F2QNZh3IguOs6n3jd/d5KwdTCk3ROoLho6Mljah6O40aq5DjvB9B\n\tdiO0BTItAhnsTnpCNzqpxmReCxzX2b1U0LO6wj59qNMPrhXs0q4HAXRPGXR/BH9wIe\n\tEu6Pr30cx6L5z5tB88srmbR/JvVwP31SatwyuHIU=","Date":"Tue, 18 Jun 2024 02:27:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","Message-ID":"<20240617232735.GG17726@pendragon.ideasonboard.com>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>\n\t<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30020,"web_url":"https://patchwork.libcamera.org/comment/30020/","msgid":"<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>","date":"2024-06-17T23:54:53","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. június 18., kedd 0:37 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta:\n\n> Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> > `__open64_2()` and `__openat64_2()` should redirect\n> > to `open64()` and `openat64()`, respectively, otherwise\n> > the `O_LARGEFILE` flag will not be applied.\n> >\n> > Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> \n> CI is all green here.\n> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630\n> \n> But commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> states:\n> \n>     Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n>     mmap symbols that we export to not be the 64-bit versions, our dlsym to\n>     get the original open and mmap calls will not automatically be converted\n>     to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n>     versions of open and mmap, we should be using the 64-bit version to\n>     service both. Fetch the 64-bit versions of openat and mmap directly.\n> \n> is that why these were the non-64 bit variants?\n> \n> This feels like it's \"too obvious\" ... I wonder what we missed ...\n> \n> Paul? Any insights? (It was ... a long time ago that you wrote that\n> patch)\n\nLooks like it is me who is confused. V4L2CompatManager::instance()->openat()\nindeed uses openat64 to service the call. And it would appear that both\n`openat64()` and `open64()` unconditionally add `O_LARGEFILE`, which, in hindsight,\nis the expected behaviour: https://codebrowser.dev/glibc/glibc/sysdeps/unix/sysv/linux/openat64.c.html#39\n\nIn this case, patch does not change a thing, apart from eliminating the\nvisual inconsistency that caught my eyes in the first place.\n\nHowever, this raises a different issue: a call to open()/openat() will automatically get `O_LARGEFILE`:\n\n$ cat asd.c\n#include <assert.h>\n#include <fcntl.h>\n#include <unistd.h>\n\nint main(int argc, char *argv[])\n{\n\tassert(argc == 2);\n\treturn close(open(argv[1], O_RDONLY));\n}\n$ gcc -m32 asd.c\n$ truncate -s 10G testfile\n\n$ strace -e openat ./a.out testfile\n[...]\nopenat(AT_FDCWD, \"testfile\", O_RDONLY)  = -1 EOVERFLOW (Value too large for defined data type)\n+++ exited with 255 +++\n\n$ strace -e openat -E LD_PRELOAD=.../src/v4l2/v4l2-compat.so ./a.out testfile\n[...]\nopenat(AT_FDCWD, \"testfile\", O_RDONLY|O_LARGEFILE) = 3\n+++ exited with 0 +++\n\nSo a file that shouldn't be opened can now be opened.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> \n> \n> > ---\n> >  src/v4l2/v4l2_compat.cpp | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> > index 8e2b7e92..8a44403e 100644\n> > --- a/src/v4l2/v4l2_compat.cpp\n> > +++ b/src/v4l2/v4l2_compat.cpp\n> > @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n> >\n> >  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n> >  {\n> > -       return open(path, oflag);\n> > +       return open64(path, oflag);\n> >  }\n> >  #endif\n> >\n> > @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n> >\n> >  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n> >  {\n> > -       return openat(dirfd, path, oflag);\n> > +       return openat64(dirfd, path, oflag);\n> >  }\n> >  #endif\n> >\n> > --\n> > 2.45.2\n> >\n> >\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 43C9FBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jun 2024 23:55:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 462626548B;\n\tTue, 18 Jun 2024 01:54:59 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C51161A20\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jun 2024 01:54:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"JeG/vhBp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718668496; x=1718927696;\n\tbh=U/UTMR9IykW3pvROh69wSR6eO9LiuOYZa0Q1k/w5byg=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=JeG/vhBpWs0/OdU4WqfivZN7foWL4D3vVMwd3nQgC9Mo1wdcny2JiyLsHN8bSO/M3\n\ti2bqk7oZiVmJo/c3IWHvjeMq9MiEykcydvAjxEfPAJQZPKXAo1B3H+FHEMeeYSd2FC\n\tEX0I1y6qEUa75vET5o6yWKDJMonogrAOAi2IiCuOtqpEXFyr7y20zo+09C3D28wZop\n\tbZgUp8xh6czYocl5SpkPgHZnbPZx+wqufeStfJBZVf+HRKQxvNuVtt3VhCoX/XV5go\n\tL9CGtN05afk4sk2Fx3kmJZshCdK4/4pefNAryH6F4pD2wE8cN/KCQ89qiG7A28ayds\n\tg0ML20zyC/zew==","Date":"Mon, 17 Jun 2024 23:54:53 +0000","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","Message-ID":"<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>","In-Reply-To":"<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>\n\t<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"3b21f276c78d0fc944bc3f15bed18af204b8d3b9","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30021,"web_url":"https://patchwork.libcamera.org/comment/30021/","msgid":"<20240618003709.GA21998@pendragon.ideasonboard.com>","date":"2024-06-18T00:37:09","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nOn Mon, Jun 17, 2024 at 11:54:53PM +0000, Barnabás Pőcze wrote:\n> 2024. június 18., kedd 0:37 keltezéssel, Kieran Bingham írta:\n> \n> > Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> > > `__open64_2()` and `__openat64_2()` should redirect\n> > > to `open64()` and `openat64()`, respectively, otherwise\n> > > the `O_LARGEFILE` flag will not be applied.\n> > >\n> > > Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > \n> > CI is all green here.\n> > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630\n> > \n> > But commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > states:\n> > \n> >     Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n> >     mmap symbols that we export to not be the 64-bit versions, our dlsym to\n> >     get the original open and mmap calls will not automatically be converted\n> >     to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n> >     versions of open and mmap, we should be using the 64-bit version to\n> >     service both. Fetch the 64-bit versions of openat and mmap directly.\n> > \n> > is that why these were the non-64 bit variants?\n> > \n> > This feels like it's \"too obvious\" ... I wonder what we missed ...\n> > \n> > Paul? Any insights? (It was ... a long time ago that you wrote that\n> > patch)\n> \n> Looks like it is me who is confused. V4L2CompatManager::instance()->openat()\n> indeed uses openat64 to service the call.\n\nThat's correct.\n\n> And it would appear that both\n> `openat64()` and `open64()` unconditionally add `O_LARGEFILE`, which, in hindsight,\n> is the expected behaviour: https://codebrowser.dev/glibc/glibc/sysdeps/unix/sysv/linux/openat64.c.html#39\n\nAahhh I was missing that part.\n\n> In this case, patch does not change a thing, apart from eliminating the\n> visual inconsistency that caught my eyes in the first place.\n\nIt also adds O_LARGEFILE manually inside the V4L2 compat layer, which\navoids confusion, even if it doesn't change the behaviour.\n\nNote that, while uclibc-ng seems to mimic the glibc implementation, musl\ndoesn't add O_LARGEFILE internally, so specifying it explicitly in the\ncompat layer is needed there.\n\n> However, this raises a different issue: a call to open()/openat() will automatically get `O_LARGEFILE`:\n> \n> $ cat asd.c\n> #include <assert.h>\n> #include <fcntl.h>\n> #include <unistd.h>\n> \n> int main(int argc, char *argv[])\n> {\n> \tassert(argc == 2);\n> \treturn close(open(argv[1], O_RDONLY));\n> }\n> $ gcc -m32 asd.c\n> $ truncate -s 10G testfile\n> \n> $ strace -e openat ./a.out testfile\n> [...]\n> openat(AT_FDCWD, \"testfile\", O_RDONLY)  = -1 EOVERFLOW (Value too large for defined data type)\n> +++ exited with 255 +++\n> \n> $ strace -e openat -E LD_PRELOAD=.../src/v4l2/v4l2-compat.so ./a.out testfile\n> [...]\n> openat(AT_FDCWD, \"testfile\", O_RDONLY|O_LARGEFILE) = 3\n> +++ exited with 0 +++\n> \n> So a file that shouldn't be opened can now be opened.\n\nIndeed. To solve that I think we would need to dlsym() the openat\nsymbol, and call it for the 32-bit calls. I wonder if this issue is\nworth fixing though.\n\nI still think your patch is worth merging, but with an updated commit\nmessage.\n\n> > > ---\n> > >  src/v4l2/v4l2_compat.cpp | 4 ++--\n> > >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> > > index 8e2b7e92..8a44403e 100644\n> > > --- a/src/v4l2/v4l2_compat.cpp\n> > > +++ b/src/v4l2/v4l2_compat.cpp\n> > > @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n> > >\n> > >  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n> > >  {\n> > > -       return open(path, oflag);\n> > > +       return open64(path, oflag);\n> > >  }\n> > >  #endif\n> > >\n> > > @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n> > >\n> > >  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n> > >  {\n> > > -       return openat(dirfd, path, oflag);\n> > > +       return openat64(dirfd, path, oflag);\n> > >  }\n> > >  #endif\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3A122C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 Jun 2024 00:37:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B7B26548E;\n\tTue, 18 Jun 2024 02:37:34 +0200 (CEST)","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 1756865456\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jun 2024 02:37:32 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 83268564;\n\tTue, 18 Jun 2024 02:37:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"caBn27Q5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718671034;\n\tbh=rWhm/1aAGbAfcIZsF6xdVoMplyww4ecb0/fxWWNBjzw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=caBn27Q5nNOaKAIagPhQphi56KJPME+v2M7ar8algjpUB9SjH7BbnsgAZZOL9r/2f\n\tNt0FTJAo6Oq/kt34z9b2i5K4nsJemty4okuwZSTdsxsXRxvLSRtR6Y46uckrcwIUBx\n\tTMn6Z5vIUaOyiPV+KsL1uHbCxuan5LeUDZOf2Iws=","Date":"Tue, 18 Jun 2024 03:37:09 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","Message-ID":"<20240618003709.GA21998@pendragon.ideasonboard.com>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>\n\t<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>\n\t<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30022,"web_url":"https://patchwork.libcamera.org/comment/30022/","msgid":"<Xt0Pjm70M08lUGJuxSiXcdCS8lzawZVrzJo0XgVdZJsQSpA8RrVhERUlGc1pyeZaBvUdCLBAcqkrMCU4LRl_xr8I--pk0epr5FYHJM1tE1Q=@protonmail.com>","date":"2024-06-18T01:23:35","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. június 18., kedd 2:37 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:\n\n> Hi Barnabás,\n> \n> On Mon, Jun 17, 2024 at 11:54:53PM +0000, Barnabás Pőcze wrote:\n> > 2024. június 18., kedd 0:37 keltezéssel, Kieran Bingham írta:\n> >\n> > > Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> > > > `__open64_2()` and `__openat64_2()` should redirect\n> > > > to `open64()` and `openat64()`, respectively, otherwise\n> > > > the `O_LARGEFILE` flag will not be applied.\n> > > >\n> > > > Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > >\n> > > CI is all green here.\n> > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630\n> > >\n> > > But commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > > states:\n> > >\n> > >     Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n> > >     mmap symbols that we export to not be the 64-bit versions, our dlsym to\n> > >     get the original open and mmap calls will not automatically be converted\n> > >     to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n> > >     versions of open and mmap, we should be using the 64-bit version to\n> > >     service both. Fetch the 64-bit versions of openat and mmap directly.\n> > >\n> > > is that why these were the non-64 bit variants?\n> > >\n> > > This feels like it's \"too obvious\" ... I wonder what we missed ...\n> > >\n> > > Paul? Any insights? (It was ... a long time ago that you wrote that\n> > > patch)\n> >\n> > Looks like it is me who is confused. V4L2CompatManager::instance()->openat()\n> > indeed uses openat64 to service the call.\n> \n> That's correct.\n> \n> > And it would appear that both\n> > `openat64()` and `open64()` unconditionally add `O_LARGEFILE`, which, in hindsight,\n> > is the expected behaviour: https://codebrowser.dev/glibc/glibc/sysdeps/unix/sysv/linux/openat64.c.html#39\n> \n> Aahhh I was missing that part.\n> \n> > In this case, patch does not change a thing, apart from eliminating the\n> > visual inconsistency that caught my eyes in the first place.\n> \n> It also adds O_LARGEFILE manually inside the V4L2 compat layer, which\n> avoids confusion, even if it doesn't change the behaviour.\n> \n> Note that, while uclibc-ng seems to mimic the glibc implementation, musl\n> doesn't add O_LARGEFILE internally, so specifying it explicitly in the\n> compat layer is needed there.\n\nWhere do you see that? I just checked, and I am fairly sure it does.\n\nhttps://git.musl-libc.org/cgit/musl/tree/src/fcntl/open.c#n16\nhttps://git.musl-libc.org/cgit/musl/tree/src/internal/syscall.h#n377\n\nAs far as I am aware, musl has always had 64-bit `off_t` and always added `O_LARGEFILE`.\n\n\n> \n> > However, this raises a different issue: a call to open()/openat() will automatically get `O_LARGEFILE`:\n> >\n> > $ cat asd.c\n> > #include <assert.h>\n> > #include <fcntl.h>\n> > #include <unistd.h>\n> >\n> > int main(int argc, char *argv[])\n> > {\n> > \tassert(argc == 2);\n> > \treturn close(open(argv[1], O_RDONLY));\n> > }\n> > $ gcc -m32 asd.c\n> > $ truncate -s 10G testfile\n> >\n> > $ strace -e openat ./a.out testfile\n> > [...]\n> > openat(AT_FDCWD, \"testfile\", O_RDONLY)  = -1 EOVERFLOW (Value too large for defined data type)\n> > +++ exited with 255 +++\n> >\n> > $ strace -e openat -E LD_PRELOAD=.../src/v4l2/v4l2-compat.so ./a.out testfile\n> > [...]\n> > openat(AT_FDCWD, \"testfile\", O_RDONLY|O_LARGEFILE) = 3\n> > +++ exited with 0 +++\n> >\n> > So a file that shouldn't be opened can now be opened.\n> \n> Indeed. To solve that I think we would need to dlsym() the openat\n> symbol, and call it for the 32-bit calls. I wonder if this issue is\n> worth fixing though.\n> \n> I still think your patch is worth merging, but with an updated commit\n> message.\n\nMaybe something like this?\n\n  To avoid confusion, have `__open64_2()` and `__openat64_2()` delegate to\n  `open64()` and `openat64()`, respectively, instead of `open()` and `open64()`.\n\n  This does not change the behaviour because `V4L2CompatManager::instance()->openat()`\n  calls `openat64()` internally, and that adds the `O_LARGEFILE` flag unconditionally.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> > > > ---\n> > > >  src/v4l2/v4l2_compat.cpp | 4 ++--\n> > > >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> > > > index 8e2b7e92..8a44403e 100644\n> > > > --- a/src/v4l2/v4l2_compat.cpp\n> > > > +++ b/src/v4l2/v4l2_compat.cpp\n> > > > @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n> > > >\n> > > >  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n> > > >  {\n> > > > -       return open(path, oflag);\n> > > > +       return open64(path, oflag);\n> > > >  }\n> > > >  #endif\n> > > >\n> > > > @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n> > > >\n> > > >  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n> > > >  {\n> > > > -       return openat(dirfd, path, oflag);\n> > > > +       return openat64(dirfd, path, oflag);\n> > > >  }\n> > > >  #endif\n> > > >\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8A2AAC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 Jun 2024 01:23:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54A20654A5;\n\tTue, 18 Jun 2024 03:23:43 +0200 (CEST)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ECA6A619F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 Jun 2024 03:23:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"f7mihkbM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718673820; x=1718933020;\n\tbh=kOfbEzCMaGK2Ph5kbqy6uUttChfzakRuH+MVz3OgiQQ=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=f7mihkbM8m7AF97CtkHyYb4k/TobNIQ7VgZ5yKK6am1vlxZESMh1zsNIlbcIDTG9Y\n\tYh3q4tEZwOPcpSV63DG8AVmabD3sxWEBsvSiKTd7PVJRnN2/ErmQlRaBHlINZ1ZfUN\n\tXtKim8sZnUT/xn4gDJl1TCUKKZ3ZONNMjwZ9N5+O9mN/CfEHSQeWnPNaGq+js3ntc+\n\tLvo5VL5grHEOGy+yxEEhhcXGHZPZH0KiuYDV9jJbLwsEy0dr/tQajMn4a01B9MasLo\n\tdxkCdlEmSVypOwdviOr4Op4wotIN00cXmrGzB7JHq45TPkel6TJ4kdbHLAh028VTN9\n\tNSNArXbyqhGfw==","Date":"Tue, 18 Jun 2024 01:23:35 +0000","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","Message-ID":"<Xt0Pjm70M08lUGJuxSiXcdCS8lzawZVrzJo0XgVdZJsQSpA8RrVhERUlGc1pyeZaBvUdCLBAcqkrMCU4LRl_xr8I--pk0epr5FYHJM1tE1Q=@protonmail.com>","In-Reply-To":"<20240618003709.GA21998@pendragon.ideasonboard.com>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>\n\t<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>\n\t<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>\n\t<20240618003709.GA21998@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"6506107c392005c5ddf6757dda7799217d54cad7","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30067,"web_url":"https://patchwork.libcamera.org/comment/30067/","msgid":"<20240625063752.GA29977@pendragon.ideasonboard.com>","date":"2024-06-25T06:37:52","subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nOn Tue, Jun 18, 2024 at 01:23:35AM +0000, Barnabás Pőcze wrote:\n> 2024. június 18., kedd 2:37 keltezéssel, Laurent Pinchart írta:\n> > On Mon, Jun 17, 2024 at 11:54:53PM +0000, Barnabás Pőcze wrote:\n> > > 2024. június 18., kedd 0:37 keltezéssel, Kieran Bingham írta:\n> > > > Quoting Barnabás Pőcze (2024-06-17 22:43:45)\n> > > > > `__open64_2()` and `__openat64_2()` should redirect\n> > > > > to `open64()` and `openat64()`, respectively, otherwise\n> > > > > the `O_LARGEFILE` flag will not be applied.\n> > > > >\n> > > > > Fixes: 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n> > > >\n> > > > CI is all green here.\n> > > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203630\n> > > >\n> > > > But commit 1023107b6405 (\"v4l2: v4l2_compat: Intercept open64, openat64, and mmap64\")\n> > > > states:\n> > > >\n> > > >     Also, since we set _FILE_OFFSET_BITS to 32 to force the various open and\n> > > >     mmap symbols that we export to not be the 64-bit versions, our dlsym to\n> > > >     get the original open and mmap calls will not automatically be converted\n> > > >     to their 64-bit versions. Since we intercept both 32-bit and 64-bit\n> > > >     versions of open and mmap, we should be using the 64-bit version to\n> > > >     service both. Fetch the 64-bit versions of openat and mmap directly.\n> > > >\n> > > > is that why these were the non-64 bit variants?\n> > > >\n> > > > This feels like it's \"too obvious\" ... I wonder what we missed ...\n> > > >\n> > > > Paul? Any insights? (It was ... a long time ago that you wrote that\n> > > > patch)\n> > >\n> > > Looks like it is me who is confused. V4L2CompatManager::instance()->openat()\n> > > indeed uses openat64 to service the call.\n> > \n> > That's correct.\n> > \n> > > And it would appear that both\n> > > `openat64()` and `open64()` unconditionally add `O_LARGEFILE`, which, in hindsight,\n> > > is the expected behaviour: https://codebrowser.dev/glibc/glibc/sysdeps/unix/sysv/linux/openat64.c.html#39\n> > \n> > Aahhh I was missing that part.\n> > \n> > > In this case, patch does not change a thing, apart from eliminating the\n> > > visual inconsistency that caught my eyes in the first place.\n> > \n> > It also adds O_LARGEFILE manually inside the V4L2 compat layer, which\n> > avoids confusion, even if it doesn't change the behaviour.\n> > \n> > Note that, while uclibc-ng seems to mimic the glibc implementation, musl\n> > doesn't add O_LARGEFILE internally, so specifying it explicitly in the\n> > compat layer is needed there.\n> \n> Where do you see that? I just checked, and I am fairly sure it does.\n> \n> https://git.musl-libc.org/cgit/musl/tree/src/fcntl/open.c#n16\n> https://git.musl-libc.org/cgit/musl/tree/src/internal/syscall.h#n377\n\n/me checks\n\nAahhh I had missed that O_LARGEFILE was hidden in __sys_open_cp().\n\nC libraries are tricky.\n\n> As far as I am aware, musl has always had 64-bit `off_t` and always\n> added `O_LARGEFILE`.\n> \n> > > However, this raises a different issue: a call to open()/openat()\n> > > will automatically get `O_LARGEFILE`:\n> > >\n> > > $ cat asd.c\n> > > #include <assert.h>\n> > > #include <fcntl.h>\n> > > #include <unistd.h>\n> > >\n> > > int main(int argc, char *argv[])\n> > > {\n> > > \tassert(argc == 2);\n> > > \treturn close(open(argv[1], O_RDONLY));\n> > > }\n> > > $ gcc -m32 asd.c\n> > > $ truncate -s 10G testfile\n> > >\n> > > $ strace -e openat ./a.out testfile\n> > > [...]\n> > > openat(AT_FDCWD, \"testfile\", O_RDONLY)  = -1 EOVERFLOW (Value too large for defined data type)\n> > > +++ exited with 255 +++\n> > >\n> > > $ strace -e openat -E LD_PRELOAD=.../src/v4l2/v4l2-compat.so ./a.out testfile\n> > > [...]\n> > > openat(AT_FDCWD, \"testfile\", O_RDONLY|O_LARGEFILE) = 3\n> > > +++ exited with 0 +++\n> > >\n> > > So a file that shouldn't be opened can now be opened.\n> > \n> > Indeed. To solve that I think we would need to dlsym() the openat\n> > symbol, and call it for the 32-bit calls. I wonder if this issue is\n> > worth fixing though.\n> > \n> > I still think your patch is worth merging, but with an updated commit\n> > message.\n> \n> Maybe something like this?\n> \n>   To avoid confusion, have `__open64_2()` and `__openat64_2()` delegate to\n>   `open64()` and `openat64()`, respectively, instead of `open()` and `open64()`.\n> \n>   This does not change the behaviour because `V4L2CompatManager::instance()->openat()`\n>   calls `openat64()` internally, and that adds the `O_LARGEFILE` flag unconditionally.\n\nI'll use that and push your patch.\n\n> > > > > ---\n> > > > >  src/v4l2/v4l2_compat.cpp | 4 ++--\n> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp\n> > > > > index 8e2b7e92..8a44403e 100644\n> > > > > --- a/src/v4l2/v4l2_compat.cpp\n> > > > > +++ b/src/v4l2/v4l2_compat.cpp\n> > > > > @@ -59,7 +59,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)\n> > > > >\n> > > > >  LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag)\n> > > > >  {\n> > > > > -       return open(path, oflag);\n> > > > > +       return open64(path, oflag);\n> > > > >  }\n> > > > >  #endif\n> > > > >\n> > > > > @@ -90,7 +90,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)\n> > > > >\n> > > > >  LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag)\n> > > > >  {\n> > > > > -       return openat(dirfd, path, oflag);\n> > > > > +       return openat64(dirfd, path, oflag);\n> > > > >  }\n> > > > >  #endif\n> > > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7811EBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 25 Jun 2024 06:38:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 83876654AA;\n\tTue, 25 Jun 2024 08:38:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7323C619D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2024 08:38:13 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5326BEB;\n\tTue, 25 Jun 2024 08:37:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lenbBUs5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719297471;\n\tbh=xsGSGtJjo1KD2YRkFG3PKYmto1edP4Dfl6YV/mt5Xsk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lenbBUs51zYOJ0rk9G4Sgbm0+Cjfc0bUHPoMlh2WRRtFTSumFySA3tLnRPmWrX9EV\n\tzjWkgsG7WTV1LZGjHt6elK0gOaHsLyvxu2IKzi8zyhPc0h5VCATNHnTiYKqzQmIz6Q\n\tKgwsARIFSzUmphTT3aDll5ozVUb3SeBFcIcb5q+k=","Date":"Tue, 25 Jun 2024 09:37:52 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] v4l2: v4l2_compat: Fix redirect from\n\t`__open(at)64_2()`","Message-ID":"<20240625063752.GA29977@pendragon.ideasonboard.com>","References":"<20240617214343.2302454-1-pobrn@protonmail.com>\n\t<171866387304.804094.10446095189594401734@ping.linuxembedded.co.uk>\n\t<z2pLsY-e_am-whjmPKBbArGu1fDacZZvUBYG4wYLc1NQHfCKG3NwpuyqoyeDqfGuHnaGGZg5DnY44wp9a4w5aPdMSCVMSut94GAHfKGQv9U=@protonmail.com>\n\t<20240618003709.GA21998@pendragon.ideasonboard.com>\n\t<Xt0Pjm70M08lUGJuxSiXcdCS8lzawZVrzJo0XgVdZJsQSpA8RrVhERUlGc1pyeZaBvUdCLBAcqkrMCU4LRl_xr8I--pk0epr5FYHJM1tE1Q=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<Xt0Pjm70M08lUGJuxSiXcdCS8lzawZVrzJo0XgVdZJsQSpA8RrVhERUlGc1pyeZaBvUdCLBAcqkrMCU4LRl_xr8I--pk0epr5FYHJM1tE1Q=@protonmail.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]