rofiles-fuse: also pass mode for O_RDONLY #1200
Conversation
|
Requires #1199. |
tests/libtest.sh
Outdated
| @@ -514,8 +514,15 @@ os_repository_new_commit () | |||
| cd ${test_tmpdir} | |||
| } | |||
|
|
|||
| assert_has_setfattr () { | |||
| if ! which setfattr 2>/dev/null; then | |||
| assert_no_reached "no setfattr available to determine xattr support" | |||
There was a problem hiding this comment.
Missing a t in not right? Could just use fatal.
There was a problem hiding this comment.
Thank! This is fixed in #1199.
.papr.yml
Outdated
| - ci/ci-commitmessage-submodules.sh | ||
| - ci/build-check.sh | ||
| - ci/ci-release-build.sh | ||
| - docker run -e TEST_TMPDIR -e CFLAGS -e GI_SCANNERFLAGS -e ASAN_OPTIONS |
There was a problem hiding this comment.
Hmm...the more we do this though the harder it is to migrate tests to run in e.g. Kubernetes. It's tricky; my recent attempt to use strace fault injection was blocked by the default docker seccomp policy, and going down this route would allow us to also easily disable that seccomp policy.
OTOH, ah just found this on Kubernetes seccomp.
Also what we're doing here maps to Kubernetes emptyDir volumes right?
So...dunno.
There was a problem hiding this comment.
Yeah, I understand. Though at the same time, we really should strive for maximum coverage in our CI, right? Given that the majority of use cases of ostree are not on overlay/tmpfs, it seems like a big one to get right.
I think due to the position in the stack of many of the projects hooked to PAPR, we can't expect great coverage from running inside unprivileged Docker containers. I don't expect the common pattern of provisioning a VM and running privileged code there to diminish after migrating to Kubernetes/OCP, right?
|
☔ The latest upstream changes (presumably 75150fe) made this pull request unmergeable. Please resolve the merge conflicts. |
In the `O_RDONLY` case, we were calling `openat` without a mode argument. However, it's perfectly legal (albeit unusual) to do `open(O_RDONLY|O_CREAT)`. One such application that makes use of this is `flock(1)`. This was actually caught by `_FORTIFY_SOURCE=2`, and once we run `rofiles-fuse` with `-f`, the message is clear: ``` *** invalid openat64 call: O_CREAT or O_TMPFILE without mode ***: rofiles-fuse terminated ======= Backtrace: ========= /lib64/libc.so.6(+0x7c8dc)[0x7f36d9f188dc] /lib64/libc.so.6(__fortify_fail+0x37)[0x7f36d9fbfaa7] /lib64/libc.so.6(+0x10019a)[0x7f36d9f9c19a] rofiles-fuse[0x401768] ... ``` Without `_FORTIFY_SOURCE`, the file gets created, but its mode is completely random. I ran into this while investigating coreos/rpm-ostree#1003.
jlebon
force-pushed
the
pr/rofiles-open
branch
from
ed2d2d0
to
c922c34
Compare
|
OK, now this PR is just about the |
|
☀️ Test successful - status-atomicjenkins |
In the
O_RDONLYcase, we were callingopenatwithout a modeargument. However, it's perfectly legal (albeit unusual) to do
open(O_RDONLY|O_CREAT). One such application that makes use of this isflock(1).This was actually caught by
_FORTIFY_SOURCE=2, and once we runrofiles-fusewith-f, the message is clear:Without
_FORTIFY_SOURCE, the file gets created, but its mode iscompletely random.
I ran into this while investigating coreos/rpm-ostree#1003.