Skip to content

Commit

Permalink
path-util: simplify check_x_access()
Browse files Browse the repository at this point in the history
Follow-up for ece852c.

This addresses the following comments by the Lennart:
---
hmm, so this now does two access() calls for the case where the fd is
not requested, and opens things up for races (theoretically, …). now,
the access() code path was in place for optimization, but if an optimization
is less sexy than the original (and i think it is less sexy, since more
than one syscall, and non-atomic), i think we shouldn't do the optimization.

maybe we should just always use open(O_PATH) now, and then fstat() it to
check if regular file, and then access_fd() it for checking if its executable.
  • Loading branch information
yuwata authored and poettering committed Jan 20, 2021
1 parent aac5fbf commit 888f65a
Showing 1 changed file with 11 additions and 23 deletions.
34 changes: 11 additions & 23 deletions src/basic/path-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -587,33 +587,21 @@ char* path_join_internal(const char *first, ...) {

static int check_x_access(const char *path, int *ret_fd) {
_cleanup_close_ int fd = -1;
const char *with_dash;
int r;

if (ret_fd) {
/* We need to use O_PATH because there may be executables for which we have only exec
* permissions, but not read (usually suid executables). */
fd = open(path, O_PATH|O_CLOEXEC);
if (fd < 0)
return -errno;

r = access_fd(fd, X_OK);
if (r < 0)
return r;
} else {
/* Let's optimize things a bit by not opening the file if we don't need the fd. */
if (access(path, X_OK) < 0)
return -errno;
}

with_dash = strjoina(path, "/");
/* We need to use O_PATH because there may be executables for which we have only exec
* permissions, but not read (usually suid executables). */
fd = open(path, O_PATH|O_CLOEXEC);
if (fd < 0)
return -errno;

/* If this passes, it must be a directory. */
if (access(with_dash, X_OK) >= 0)
return -EISDIR;
r = fd_verify_regular(fd);
if (r < 0)
return r;

if (errno != ENOTDIR)
return -errno;
r = access_fd(fd, X_OK);
if (r < 0)
return r;

if (ret_fd)
*ret_fd = TAKE_FD(fd);
Expand Down

0 comments on commit 888f65a

Please sign in to comment.