See description in blocked bug. I was going to just try by adding append-lfs-flags myself, but there's a bug referenced and a 'filter-lfs-flags' already in the ebuild. I don't fully understand the reason it was added, may it is invalid now? Reproducible: Always
I'd guess sandbox needs to cover both non-LFS and LFS interfaces. Somehow.
Well, I tried anyway to replace 'filter-lfs-flags' with 'append-lfs-flags' and it didn't go well. Got new warnings/errors, so I'm guessing there's still a valid reason that it's forced off for the package. I did however manage to fix/sidestep the issue by putting "#define _FILE_OFFSET_BITS 64" before any other includes in the two files that use readdir(), namely libsbutil/sb_close.c and libsbutil/src/file.c. My thinking was that since sandbox (I think) in these files is just _using_ readdir() for internal purposes, it can use the 64 bit offsets and keep that information to itself. It did get rid of the warnings I reported in the parent bug, but I'm not 100% if this is a viable solution, and/or applies to all platforms/libc's supported.
Can you provide a list of few functions that sandbox does not cover in your opinion? Say, on i386. I see both lfs and non-lfs interfaces redirected: $ nm -D /usr/lib32/libsandbox.so | fgrep -C2 64 0000ad40 T __open64_2 0000a420 T __open_2 0000ae50 T __openat64_2 0000a530 T __openat_2 ... 000072b0 T creat 000087d0 T creat64 ... 0000a390 T open 0000acb0 T open64 0000a4a0 T openat 0000adc0 T openat64 Note: sandbox intentionally does not try to convert non-lfs programs into lfs ones and just passes through whatever is ran under it.
I think part of the trouble is lack of understanding (me) of sandbox when I filed the bug. The bug description made sense when I filed but maybe not now and should maybe be updated. Other packages simply needed LFS turned on, but here because (if I am beginning to see the light) sandbox is wrapping syscalls, it doesn't simply need LFS turned on, but to have its internal calls to readdir work using LFS. IIUC, when used inside of a qemu chroot on a 64bit host, the following happens: Pre-conditions: 64bit host 32bit chroot, using a static qemu-arm When sandbox calls readdir() (for itself, not on behalf of client code), but without _FILE_OFFSET_BITS set to 64, the size of the internal dirent fields are 32bit. However, because qemu is a 64bit process, and the recent changes to glibc (see parent bug), when the call gets to the host's glibc it says "hey, you're a 64bit process, but you only gave me a 32bit off_t and ino_t". Prior to the glibc changes, it would just truncate and most code didn't notice because it isn't interested in the ino_t or off_t. However now, the call can fail outright and returns a null dirent. In sandbox you see warnings about readdir() pop up. I'm not sure how else it is affected but I assume it skips part of the code path. So I think here, the behaviour of sandbox wrapping function calls is correct, but the couple of calls it makes internally in its use of readdir() needs to have the option for 64bit file offsets. Thankfully sandbox warns you. Other packages without error checking just silently don't perform portions of their job. Hopefully I'm explaining well enough with my limited view.
https://lkml.org/lkml/2018/12/27/155
Myself and the Debian glibc maintainer have been helping test a patch-set which should hopefully correct the underlying glibc problem. https://sourceware.org/pipermail/libc-alpha/2020-April/112866.html It has stalled at the review phase, but I am poking presently.
It's still moving along v2 patchset was posted and reviewed here https://sourceware.org/pipermail/libc-alpha/2020-October/118274.html v3 patchset is pending review https://sourceware.org/pipermail/libc-alpha/2020-October/118883.html
*** Bug 762367 has been marked as a duplicate of this bug. ***
diff --git a/libsbutil/Makefile.am b/libsbutil/Makefile.am index 684d126..bf2dd1d 100644 --- a/libsbutil/Makefile.am +++ b/libsbutil/Makefile.am @@ -1,6 +1,7 @@ AUTOMAKE_OPTIONS = foreign AM_CPPFLAGS = \ + -D_FILE_OFFSET_BITS=64 \ -I$(top_srcdir) \ -I$(srcdir)/include \ $(SANDBOX_DEFINES) ^ could you please test this mini-patch? This, at least to my understanding, should force the sandbox-internal calls to always use the LFS interface while leaving the wrapped calls untouched. Tests pass with this change here; beyond that I have not tested it in real-life situations yet.
(In reply to Andreas K. Hüttel from comment #9) that doesn't quite cut it. we need to interpose LFS & non-LFS interfaces.
there's 2 distinct scenarios (1) sandbox needs to interpose LFS & non-LFS interfaces that apps might try use. from a quick check of symbols (but by no means exhaustive), i'm seeing what Sergei sees -- libsandbox is catching both interaces. if there's a specific one you think we're missing, please highlight it. (2) sandbox, internally, needs to always use LFS interfaces when checking paths. we don't do that today, and that's a bug. but we have bug 583282 for that already. based on comment #4 here by the OP, it sounds like the report is about situation (2) here, so duping over to the existing report. *** This bug has been marked as a duplicate of bug 583282 ***
(In reply to SpanKY from comment #11) > there's 2 distinct scenarios > > (1) sandbox needs to interpose LFS & non-LFS interfaces that apps might try > use. from a quick check of symbols (but by no means exhaustive), i'm seeing > what Sergei sees -- libsandbox is catching both interaces. if there's a > specific one you think we're missing, please highlight it. > > (2) sandbox, internally, needs to always use LFS interfaces when checking > paths. we don't do that today, and that's a bug. but we have bug 583282 > for that already. > > based on comment #4 here by the OP, it sounds like the report is about > situation (2) here, so duping over to the existing report. Unfortunately I only read this now when I was pointed to it. Regarding (1), I deliberately modified only libsbutil, because in my understanding that's where the internal accesses are. > > *** This bug has been marked as a duplicate of bug 583282 ***
(In reply to Andreas K. Hüttel from comment #12) > Regarding (1), I deliberately modified only libsbutil, because in my > understanding that's where the internal accesses are. libsandbox code directly touches the filesystem via stat calls. we also really really want to avoid different modules in the same codebase using different FS interfaces. if libsbutil thought "struct stat" was actually "struct stat64" and libsandbox thought "struct stat" was just "struct stat", then the compiler wouldn't complain about calls between them. it'd be a nightmare to debug and maintain.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=f4872fb69fe16fc416e4211d12811da61e8738b2 commit f4872fb69fe16fc416e4211d12811da61e8738b2 Author: Mike Frysinger <vapier@gentoo.org> AuthorDate: 2021-11-05 09:47:58 +0000 Commit: Mike Frysinger <vapier@gentoo.org> CommitDate: 2021-11-05 09:47:58 +0000 Revert "Force sandbox-internal functions to use 64bit file interface" This reverts commit 19c215f245faf9a453e7171bddccc690c03f7b72. We do not want different LFS interfaces being used in different modules as it makes debugging a nightmare when different functions think basic structures have different layouts & sizes. This also doesn't address the LFS issues sandbox has when code still crashes in libsandbox itself when checking accesses. Bug: https://bugs.gentoo.org/681892 Signed-off-by: Mike Frysinger <vapier@gentoo.org> libsbutil/local.mk | 1 - 1 file changed, 1 deletion(-)
