[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Tue, 30 Jan 2018 23:32:37 +0300
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: [PATCH] faccessat: fix error code on setreXid failure
Commit 316d6741b68b485205d7233c98bd6c795bb80370 changed one use of
SYS_exit in 'checker' without changing another just three lines above,
and then commit f9fb20b42da0e755d93de229a5a737d79a0e8f60 changed the
meaning of return value, causing EACCES to be reported instead of EBUSY
if preparatory setregid/setreuid fail.
---
This is the minimal fix for the issue, but it appears there's another:
collecting checker's exit code and reaping the zombie is implemented as
int status;
do {
__syscall(SYS_wait4, pid, &status, __WCLONE, 0);
} while (!WIFEXITED(status) && !WIFSIGNALED(status));
but I don't understand why this retry loop is required and correct:
- if another thread won the race to collect the zombie by doing something
like waitpid(-1, 0, __WALL), it fails to check syscall's return value and
uses uninitialized 'status', possibly causing an infinite loop or OOB access
in the parent;
- the code seems to assume that the zombie will not be auto-collected even if
SIGCHLD disposition is set to SIG_IGN; this sounds logical, but not explicitly
documented as far as I can tell;
- if the two problems above don't arise, I don't see how the test in while ()
condition can fail; we have signals blocked, so waitpid can only return when
the child no longer exists.
Plus, using CLONE_VM | CLONE_VFORK would help conserve resources.
Alexander
src/unistd/faccessat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/unistd/faccessat.c b/src/unistd/faccessat.c
index 33478959..954cbdb4 100644
--- a/src/unistd/faccessat.c
+++ b/src/unistd/faccessat.c
@@ -25,7 +25,7 @@ static int checker(void *p)
int i;
if (__syscall(SYS_setregid, __syscall(SYS_getegid), -1)
|| __syscall(SYS_setreuid, __syscall(SYS_geteuid), -1))
- __syscall(SYS_exit, 1);
+ return sizeof errors/sizeof *errors - 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;
--
2.11.0
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
