]> granicus.if.org Git - musl/commitdiff
revert regression in faccessat AT_EACCESS robustness
authorRich Felker <dalias@aerifal.cx>
Mon, 5 Feb 2018 16:31:11 +0000 (11:31 -0500)
committerRich Felker <dalias@aerifal.cx>
Mon, 5 Feb 2018 16:31:11 +0000 (11:31 -0500)
commit f9fb20b42da0e755d93de229a5a737d79a0e8f60 switched from using a
pipe for the result to conveying it via the child process exit status.
Alexander Monakov pointed out that the latter could fail if the
application is not expecting faccessat to produce a child and performs
a wait operation with __WCLONE or __WALL, and that it is not clear
whether it's guaranteed to work when SIGCHLD's disposition has been
set to SIG_IGN.

in addition, that commit introduced a bug that caused EACCES to be
produced instead of EBUSY due to an exit path that was overlooked when
the error channel was changed, and introduced a spurious retry loop
around the wait operation.

src/unistd/faccessat.c

index 33478959cb0edeaf08ed3860ecac04a1d5c488f3..76bbd4c72a16dbea7cdef397aa12ccb013adaa85 100644 (file)
@@ -1,6 +1,5 @@
 #include <unistd.h>
 #include <fcntl.h>
-#include <errno.h>
 #include <sys/wait.h>
 #include "syscall.h"
 #include "pthread_impl.h"
@@ -9,26 +8,19 @@ struct ctx {
        int fd;
        const char *filename;
        int amode;
-};
-
-static const int errors[] = {
-       0, -EACCES, -ELOOP, -ENAMETOOLONG, -ENOENT, -ENOTDIR,
-       -EROFS, -EBADF, -EINVAL, -ETXTBSY,
-       -EFAULT, -EIO, -ENOMEM,
-       -EBUSY
+       int p;
 };
 
 static int checker(void *p)
 {
        struct ctx *c = p;
        int ret;
-       int i;
        if (__syscall(SYS_setregid, __syscall(SYS_getegid), -1)
            || __syscall(SYS_setreuid, __syscall(SYS_geteuid), -1))
                __syscall(SYS_exit, 1);
        ret = __syscall(SYS_faccessat, c->fd, c->filename, c->amode, 0);
-       for (i=0; i < sizeof errors/sizeof *errors - 1 && ret!=errors[i]; i++);
-       return i;
+       __syscall(SYS_write, c->p, &ret, sizeof ret);
+       return 0;
 }
 
 int faccessat(int fd, const char *filename, int amode, int flag)
@@ -42,20 +34,21 @@ int faccessat(int fd, const char *filename, int amode, int flag)
        char stack[1024];
        sigset_t set;
        pid_t pid;
-       int ret = -EBUSY;
-       struct ctx c = { .fd = fd, .filename = filename, .amode = amode };
+       int status;
+       int ret, p[2];
+
+       if (pipe2(p, O_CLOEXEC)) return __syscall_ret(-EBUSY);
+       struct ctx c = { .fd = fd, .filename = filename, .amode = amode, .p = p[1] };
 
        __block_all_sigs(&set);
        
        pid = __clone(checker, stack+sizeof stack, 0, &c);
-       if (pid > 0) {
-               int status;
-               do {
-                       __syscall(SYS_wait4, pid, &status, __WCLONE, 0);
-               } while (!WIFEXITED(status) && !WIFSIGNALED(status));
-               if (WIFEXITED(status))
-                       ret = errors[WEXITSTATUS(status)];
-       }
+       __syscall(SYS_close, p[1]);
+
+       if (pid<0 || __syscall(SYS_read, p[0], &ret, sizeof ret) != sizeof(ret))
+               ret = -EBUSY;
+       __syscall(SYS_close, p[0]);
+       __syscall(SYS_wait4, pid, &status, __WCLONE, 0);
 
        __restore_sigs(&set);