SQLite User Forum

should use eaccess instead of access to check temporary file directory
Login

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 [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 [link] [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?