should use eaccess instead of access to check temporary file directory
(1) By jjm2473 on 2020-08-16 03:38:25 [link] [source]
The 'access' syscall uses REAL uid to check aceess, we should use EFFECTIVE uid with 'eaccess' call. See man page for their difference.
Detail:
Issue in forked-daapd in OpenWRT, before patched, forked-daapd scanning multi files or doing database vacuum, 'unable to open database file' may appears in log. Using strace to trace it, shows that:
setresgid(-1, 3003, -1) = 0
...
setresuid(-1, 190, -1) = 0
...
newfstatat(AT_FDCWD, "/var/tmp", {st_mode=S_IFDIR|0755, st_size=40, ...}, 0) = 0
faccessat(AT_FDCWD, "/var/tmp", W_OK|X_OK) = 0
faccessat(AT_FDCWD, "/var/tmp/etilqs_d4c84886ce52e72a", F_OK) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/var/tmp/etilqs_d4c84886ce52e72a", O_RDWR|O_CREAT|O_EXCL|O_LARGEFILE|O_NOFOLLOW|O_CLOEXEC, 0600) = -1 EACCES (Permission denied)
openat(AT_FDCWD, "/var/tmp/etilqs_d4c84886ce52e72a", O_RDONLY|O_EXCL|O_LARGEFILE|O_NOFOLLOW|O_CLOEXEC) = -1 ENOENT (No such file or directory)
As shown above, forked-daapd was run by root, then it sets its effective uid to 190,
then sqlite trying to check '/var/tmp' writeable with faccessat(AT_FDCWD, "/var/tmp", W_OK|X_OK)
, kernel says ok, but failed when actually create file with openat
.
Because faccessat
using real uid to check access by default, but openat
using effective uid.
PS, The information of '/var/tmp' :
drwxr-xr-x 2 root root 40 Aug 16 10:29 tmp
Patch:
--- a/sqlite3.c
+++ b/sqlite3.c
@@ -33842,7 +33842,7 @@ static struct unix_syscall {
{ "close", (sqlite3_syscall_ptr)close, 0 },
#define osClose ((int(*)(int))aSyscall[1].pCurrent)
- { "access", (sqlite3_syscall_ptr)access, 0 },
+ { "access", (sqlite3_syscall_ptr)eaccess, 0 },
#define osAccess ((int(*)(const char*,int))aSyscall[2].pCurrent)
{ "getcwd", (sqlite3_syscall_ptr)getcwd, 0 },
(2) By Stephan Beal (stephan) on 2020-08-16 03:43:49 in reply to 1 [link] [source]
man eaccess
says it's not a standard API...
NAME
euidaccess, eaccess - check effective user's permissions for a file
SYNOPSIS
#define _GNU_SOURCE /* See feature_test_macros(7) */
#include <unistd.h>
...
CONFORMING TO
These functions are nonstandard. Some other systems have an eaccess() function.
(3) By jjm2473 on 2020-08-16 10:13:40 in reply to 2 [source]
No, eaccess is NOT portable. Maybe we should find solution in libc.
In musl libc, eaccess is define as:
euidaccess.c
#define _GNU_SOURCE
#include <unistd.h>
#include <fcntl.h>
#include "libc.h"
int euidaccess(const char *filename, int amode)
{
return faccessat(AT_FDCWD, filename, amode, AT_EACCESS);
}
weak_alias(euidaccess, eaccess);
faccessat.c:
#include <unistd.h>
#include <fcntl.h>
#include <sys/wait.h>
#include "syscall.h"
#include "pthread_impl.h"
struct ctx {
int fd;
const char *filename;
int amode;
int p;
};
static int checker(void *p)
{
struct ctx *c = p;
int ret;
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);
__syscall(SYS_write, c->p, &ret, sizeof ret);
return 0;
}
int faccessat(int fd, const char *filename, int amode, int flag)
{
if (!flag || (flag==AT_EACCESS && getuid()==geteuid() && getgid()==getegid()))
return syscall(SYS_faccessat, fd, filename, amode, flag);
if (flag != AT_EACCESS)
return __syscall_ret(-EINVAL);
char stack[1024];
sigset_t set;
pid_t pid;
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);
__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);
return __syscall_ret(ret);
}
So, it calls clone to run checker.
But is clone
a good way? (Some system may strict clone syscall)
Do you have other ways?
Can we just change unixTempFileDir
to really create a temporary file to check the directory is writable?