Skip to content

Conversation

shaharlib
Copy link
Contributor

When the overlay driver (and some others) tries to access the directories for the relevant layers it uses faccessat syscall (in fileutils.Exists() fileutils.Lexists()).
The syscall is missing the AT_EACCESS flag and so permissions to access the files are determined solely on the uid.
This error happens when we run with a user that has the correct capabilities to access the file (e.g cap_dac_override) but the file permissions don't allow the specific user.

This PR adds the AT_EACCESS flag to the faccessat syscall in Exists() and Lexists() functions to ensure that they will be able to verify file existence based on the effective user.

How to reproduce (adding a binary to help demonstrate the issue)

Compile this faccessast_test program

#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <sys/stat.h>
int main(int argc, char *argv[]) {
    if (argc != 2) {
        fprintf(stderr, "Usage: %s <path>\n", argv[0]);
        return 1;
    }
    const char *path = argv[1];
    
    if (faccessat(AT_FDCWD, path, F_OK, AT_SYMLINK_NOFOLLOW) == 0) {
        printf("[faccessat][without AT_EACCESS]\tFile exists: %s\n", path);
    } else {
        perror("[faccessat][without AT_EACCESS]");
    }
    
    if (faccessat(AT_FDCWD, path, F_OK, AT_SYMLINK_NOFOLLOW|AT_EACCESS) == 0) {
        printf("[faccessat][with AT_EACCESS]\tFile exists: %s\n", path);
    } else {
        perror("[faccessat][with AT_EACCESS]");
    }
    return 0;
}

Then use the following commands:

sudo mkdir -p /foo/a
sudo chmod -R 700 /foo/
sudo chown -R root:root /foo/
sudo setcap "cap_dac_override=eip" faccessat_test
./faccessat_test /foo/a

…access layers.

When the overlay driver (and some others) tries to access the directories for the relevant layers it uses faccessat syscall (in fileutils.Exists() fileutils.Lexists()).
The syscall is missing the AT_EACCESS flag and so permissions to access the files are determined solely on the uid.
This error happens when we run with a user that has the correct capabilities to access the file (e.g cap_dac_override) but the file permissions don't allow the specific user.

This commit adds the AT_EACCESS flag to the faccessat syscall in Exists() and Lexists() functions to ensure that they will be able to verify file existence based on the effective user.

Signed-off-by: Shahar Liberman <shahar.liberman@wiz.io>
@shaharlib shaharlib force-pushed the shaharlib/fix_permission_error_use_at_eaccess_flag_faccessat branch from ba5a5d2 to a6d2ba2 Compare March 20, 2025 12:31
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but @giuseppe PTAL to make sure I'm not missing anything.

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, the patch LGTM

/lgtm

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

openshift-ci bot commented Mar 21, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, kolyshkin, shaharlib

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 92bf4f2 into containers:main Mar 21, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants