On Tue, Sep 20, 2022 at 5:15 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Sep 19, 2022 at 10:21 PM Jon Doron <arilou@xxxxxxxxx> wrote:
> >
> > From: Jon Doron <jond@xxxxxx>
> >
> > When running rootless with special capabilities like:
> > FOWNER / DAC_OVERRIDE / DAC_READ_SEARCH
> >
> > The access API will not make the proper check if there is really
> > access to a file or not.
> >
>
> This is very succinct and doesn't explain why access() doesn't work. I
> had to read the man page for access() to (hopefully) understand what's
> going on. Please elaborate a bit more and maybe quote man page:
>
> The check is done using the calling process's real UID and GID,
> rather than the effective IDs as is done when actually attempting
> an operation (e.g., open(2)) on the file. Similarly, for the
> root user, the check uses the set of permitted capabilities
> rather than the set of effective capabilities; and for non-root
> users, the check uses an empty set of capabilities.
>
> This allows set-user-ID programs and capability-endowed programs
> to easily determine the invoking user's authority. In other
> words, access() does not answer the "can I read/write/execute
> this file?" question. It answers a slightly different question:
> "(assuming I'm a setuid binary) can the user who invoked me
> read/write/execute this file?", which gives set-user-ID programs
> the possibility to prevent malicious users from causing them to
> read files which users shouldn't be able to read.
>
> So if I understand correctly, access() is self-limiting itself
> artificially, while in practice target file can be totally readable
> due to caps or effective user ID differences.
>
> Please try to summarize this in the commit message.
>
Oh, and I think it's fine to target bpf-next tree with this. Also
please prefix libbpf patches with "libbpf: ", so your patch subject
should start with "[PATCH v2 bpf-next] libbpf: ..."
> > Signed-off-by: Jon Doron <jond@xxxxxx>
> > ---
> > tools/lib/bpf/libbpf.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 50d41815f431..df804fd65493 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -875,8 +875,9 @@ __u32 get_kernel_version(void)
> > const char *ubuntu_kver_file = "/proc/version_signature";
> > __u32 major, minor, patch;
> > struct utsname info;
> > + struct stat sb;
> >
> > - if (access(ubuntu_kver_file, R_OK) == 0) {
> > + if (stat(ubuntu_kver_file, &sb) == 0) {
> > FILE *f;
> >
> > f = fopen(ubuntu_kver_file, "r");
> > @@ -9877,9 +9878,10 @@ static int append_to_file(const char *file, const char *fmt, ...)
> > static bool use_debugfs(void)
> > {
> > static int has_debugfs = -1;
> > + struct stat sb;
> >
> > if (has_debugfs < 0)
> > - has_debugfs = access(DEBUGFS, F_OK) == 0;
> > + has_debugfs = stat(DEBUGFS, &sb) == 0;
> >
>
> I found in total 5 access() uses in libbpf, can you please update the
> other 3 as well? Thanks!
>
>
> > return has_debugfs == 1;
> > }
> > --
> > 2.37.3
> >
