Add more O_PATH flags

[eriksjolund]

trafficstars

Currently WIP (work in progress). I haven't done any benchmarks yet.

Fixes: https://github.com/containers/crun/issues/1478

[eriksjolund]

I compiled two crun executables, one with and one without this PR. The gcc flag -O2 was used. Then I tried

podman pull docker.io/library/busybox
hyperfine -w 10 -m 100 "podman --runtime /home/test/crun.with_PR --pull=never docker.io/library/busybox true"
hyperfine -w 10 -m 100 "podman --runtime /home/test/crun.without_PR --pull=never docker.io/library/busybox true"

I didn't see any consistent speedup. When I repeated the commands, the results were a bit random. Sometimes /home/test/crun.with_PR was a few percentage faster and sometimes /home/test/crun.without_PR was a few percentage faster. Sometimes they were about equally fast.

Anyway, it seems this PR does not improve performance in any significant way.

[eriksjolund]

Quote from https://man7.org/linux/man-pages/man2/open.2.html

When O_PATH is specified in flags, flag bits other than
O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored.

Because of that I removed O_RDONLY and pushed a new version.

Do you have any feedback about the usefulness of this PR?

You could also close this PR if this PR has little benefit.

Side note: I saw a similar PR for systemd:

• https://github.com/systemd/systemd/pull/33591

[flouthoc]

@eriksjolund I am not sure but isn't O_PATH more suitable when file data is not important, I am worried if this will give permission errors at misc spots where crun is trying to read the data and maybe tests are not covering those branches ?

Is swapping O_RDONLY with O_PATH giving significant performance gains ?

[eriksjolund]

@flouthoc Yes, you have point. I also came to think that permission denied errors might occur at other places when starting to use O_PATH. I think I will just to close the PR (and the issue) in a few days unless anyone disagrees.

[eriksjolund]

Is swapping O_RDONLY with O_PATH giving significant performance gains ?

quote from https://github.com/systemd/systemd/pull/33591#issue-2388245128

This because the kernel code path stops very early when this flag is given:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/open.c?h=v6.9#n920

Stopping early should be faster but I guess the difference is insignificant. At least I couldn't detect any consistent speedup when I did some benchmarks.

[flouthoc]

@flouthoc Yes, you have point. I also came to think that permission denied errors might occur at other places when starting to use O_PATH. I think I will just to close the PR (and the issue) in a few days unless anyone disagrees.

SGTM

[flouthoc]

My bad I closed the PR by mistake, please wait for other reviewers before closing the PR. @giuseppe WDYT ?

[kolyshkin]

To me, the performance benefit is insignificant, but the security aspect is. Although I can not think of any specific case where not using O_PATH will result in a potential security issue, I can say that when using O_PATH, we get even less permissions than O_RDONLY gives us, and generally less permissions is good.

I would add O_NOFOLLOW in a few cases, too.

[rhatdan]

Waiting for @giuseppe to merge. It would be nice to run this through Podman or CRI-O tests, but I don't think that is all that easy to do.