Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 26 May 2014 00:28:57 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [UGLY PATCH] Support for no-legacy-syscalls archs

On Sun, May 25, 2014 at 01:42:37AM -0400, Rich Felker wrote:
> Here's a proposed next phase for supporting no-legacy-syscall archs
> (aarch64 and or1k, among others). It's not complete but I think it
> covers most of the important syscalls for standard functionality (not
> linux-specific stuff tho). Some of them might be missing some error
> cases or otherwise buggy so I'm sending the patch for review before
> committing.

Here's a list of all the syscalls we need to (conditionally) avoid:

/* non-at */
#define __NR_open 1024
#define __NR_link 1025
#define __NR_unlink 1026
#define __NR_mknod 1027
#define __NR_chmod 1028
#define __NR_chown 1029
#define __NR_mkdir 1030
#define __NR_rmdir 1031
#define __NR_lchown 1032
#define __NR_access 1033
#define __NR_rename 1034
#define __NR_readlink 1035
#define __NR_symlink 1036
#define __NR_utimes 1037
#define __NR3264_stat 1038
#define __NR3264_lstat 1039
/* non-flags */
#define __NR_pipe 1040
#define __NR_dup2 1041
#define __NR_epoll_create 1042
#define __NR_inotify_init 1043
#define __NR_eventfd 1044
#define __NR_signalfd 1045
/* deprecated */
#define __NR_alarm 1059
#define __NR_getpgrp 1060
#define __NR_pause 1061
#define __NR_time 1062
#define __NR_utime 1063
#define __NR_creat 1064
#define __NR_getdents 1065
#define __NR_futimesat 1066
#define __NR_select 1067
#define __NR_poll 1068
#define __NR_epoll_wait 1069
#define __NR_ustat 1070
#define __NR_vfork 1071
#define __NR_oldwait4 1072
#define __NR_recv 1073
#define __NR_send 1074
#define __NR_bdflush 1075
#define __NR_umount 1076
#define __NR_uselib 1077
#define __NR__sysctl 1078
#define __NR_fork 1079

With the changes so far, the only syscalls listed above that aren't
addressed yet are the ones for linux-specific features (epoll,
signalfd, etc.) and utimes, I think.

> diff --git a/src/select/poll.c b/src/select/poll.c
> index f1e73e8..4f13cfb 100644
> --- a/src/select/poll.c
> +++ b/src/select/poll.c
> @@ -4,5 +4,11 @@
>  
>  int poll(struct pollfd *fds, nfds_t n, int timeout)
>  {
> +#ifdef SYS_poll
>  	return syscall_cp(SYS_poll, fds, n, timeout);
> +#else
> +	return syscall_cp(SYS_ppoll, fds, n,
> +		&(struct timespec){ .tv_sec = timeout/1000,
> +		.tv_nsec = timeout%1000*1000000 }, 0, _NSIG/8);
> +#endif

This may be wrong with negative values for timeout.

> diff --git a/src/select/select.c b/src/select/select.c
> index f93597b..5e7fe1f 100644
> --- a/src/select/select.c
> +++ b/src/select/select.c
> @@ -4,5 +4,15 @@
>  
>  int select(int n, fd_set *restrict rfds, fd_set *restrict wfds, fd_set *restrict efds, struct timeval *restrict tv)
>  {
> +#ifdef SYS_select
>  	return syscall_cp(SYS_select, n, rfds, wfds, efds, tv);
> +#else
> +	long data[2] = { 0, _NSIG/8 };
> +	struct timespec ts;
> +	if (tv) {
> +		ts.tv_sec = tv->tv_sec;
> +		ts.tv_nsec = tv->tv_usec * 1000;
> +	}
> +	return syscall_cp(SYS_pselect6, n, rfds, wfds, efds, tv ? &ts : 0, data);
> +#endif

This can overflow if tv_usec is nonsense.

> diff --git a/src/stat/futimesat.c b/src/stat/futimesat.c
> index dbefc84..ad33484 100644
> --- a/src/stat/futimesat.c
> +++ b/src/stat/futimesat.c
> @@ -2,9 +2,18 @@
>  #include <sys/time.h>
>  #include "syscall.h"
>  
> -#ifdef SYS_futimesat
>  int futimesat(int dirfd, const char *pathname, const struct timeval times[2])
>  {
> +#ifdef SYS_futimesat
>  	return syscall(SYS_futimesat, dirfd, pathname, times);
> -}
> +#else
> +	struct timespec ts[2];
> +	if (times) {
> +		ts[0].tv_sec = times[0].tv_sec;
> +		ts[0].tv_nsec = times[0].tv_usec * 1000;
> +		ts[1].tv_sec = times[1].tv_sec;
> +		ts[1].tv_nsec = times[1].tv_usec * 1000;
> +	}
> +	return syscall(SYS_utimensat, dirfd, pathname, times ? ts : 0, 0);
>  #endif

This is one of the cases where the legacy syscall really is junk and
we should always be using the modern one, when available. I'd like to
make futimesat, utimes, and utime _all_ call utimensat, and put the
fallback logic for using legacy syscalls when utimensat failes with
ENOSYS all in one place.

> diff --git a/src/unistd/dup2.c b/src/unistd/dup2.c
> index 87a0d44..7bfcaf2 100644
> --- a/src/unistd/dup2.c
> +++ b/src/unistd/dup2.c
> @@ -5,6 +5,15 @@
>  int dup2(int old, int new)
>  {
>  	int r;
> +#ifdef SYS_dup2
>  	while ((r=__syscall(SYS_dup2, old, new))==-EBUSY);
> +#else
> +	if (old==new) {
> +		if (__syscall(SYS_fcntl, old, F_GETFD) < 0)
> +			return __syscall_ret(-EBADF);
> +		return 0;
> +	}
> +	while ((r=__syscall(SYS_dup3, old, new, 0))==-EBUSY);
> +#endif
>  	return __syscall_ret(r);
>  }

I have a new approach to this that's less bloated: after the syscall
returns, check for r==-EINVAL && old==new, and in that case, return
new rather than an error.

> diff --git a/src/unistd/getpgrp.c b/src/unistd/getpgrp.c
> index 433f42e..2c8aae6 100644
> --- a/src/unistd/getpgrp.c
> +++ b/src/unistd/getpgrp.c
> @@ -1,7 +1,16 @@
>  #include <unistd.h>
>  #include "syscall.h"
> +#include "pthread_impl.h"
>  
>  pid_t getpgrp(void)
>  {
> +#ifdef SYS_getpgrp
>  	return __syscall(SYS_getpgrp);
> +#else
> +	sigset_t set;
> +	__block_app_sigs(&set);
> +	pid_t ret = __syscall(SYS_getpgid, __syscall(SYS_getpid));
> +	__restore_sigs(&set);
> +	return ret;
> +#endif

This can simply be __syscall(SYS_getpgid, 0). And that's so simple
it's not even worth having the #ifdef; we should just avoid the legacy
syscall unconditionally.

> diff --git a/src/unistd/pause.c b/src/unistd/pause.c
> index f7ed17d..50bf3b1 100644
> --- a/src/unistd/pause.c
> +++ b/src/unistd/pause.c
> @@ -4,5 +4,11 @@
>  
>  int pause(void)
>  {
> +#ifdef SYS_pause
>  	return syscall_cp(SYS_pause);
> +#else
> +	sigset_t mask;
> +	__syscall(SYS_rt_sigprocmask, SIG_BLOCK, 0, &mask, _NSIG/8);
> +	return syscall_cp(SYS_rt_sigsuspend, &mask, _NSIG/8);
> +#endif

I think this is correct, but it could use a check. SYS_pause is also
used in exit.c but it doesn't really need to be...

Rich

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.