From 4ddbb8a9739aa00a808d2e9605a9854345223cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kowalczyk?= Date: Thu, 9 Apr 2020 18:25:06 +0200 Subject: [PATCH] [LibOS] Fix handling of dup* with invalid args --- LibOS/shim/include/shim_table.h | 12 ++++----- LibOS/shim/src/bookkeep/shim_handle.c | 39 +++++++++++++++------------ LibOS/shim/src/shim_syscalls.c | 7 ++--- LibOS/shim/src/sys/shim_dup.c | 23 +++++++++------- LibOS/shim/test/ltp/ltp.cfg | 17 ------------ 5 files changed, 46 insertions(+), 52 deletions(-) diff --git a/LibOS/shim/include/shim_table.h b/LibOS/shim/include/shim_table.h index 166b2d0e..b399da12 100644 --- a/LibOS/shim/include/shim_table.h +++ b/LibOS/shim/include/shim_table.h @@ -353,8 +353,8 @@ int shim_do_sched_yield(void); void* shim_do_mremap(void* addr, size_t old_len, size_t new_len, int flags, void* new_addr); int shim_do_msync(void* start, size_t len, int flags); int shim_do_mincore(void* start, size_t len, unsigned char* vec); -int shim_do_dup(int fd); -int shim_do_dup2(int oldfd, int newfd); +int shim_do_dup(unsigned int fd); +int shim_do_dup2(unsigned int oldfd, unsigned int newfd); int shim_do_pause(void); int shim_do_nanosleep(const struct __kernel_timespec* rqtp, struct __kernel_timespec* rmtp); int shim_do_getitimer(int which, struct __kernel_itimerval* value); @@ -486,7 +486,7 @@ int shim_do_get_robust_list(pid_t pid, struct robust_list_head** head, size_t* l int shim_do_epoll_pwait(int epfd, struct __kernel_epoll_event* events, int maxevents, int timeout_ms, const __sigset_t* sigmask, size_t sigsetsize); int shim_do_accept4(int sockfd, struct sockaddr* addr, socklen_t* addrlen, int flags); -int shim_do_dup3(int oldfd, int newfd, int flags); +int shim_do_dup3(unsigned int oldfd, unsigned int newfd, int flags); int shim_do_epoll_create1(int flags); int shim_do_pipe2(int* fildes, int flags); ssize_t shim_do_recvmmsg(int sockfd, struct mmsghdr* msg, size_t vlen, int flags, @@ -541,8 +541,8 @@ int shim_madvise(void* start, size_t len, int behavior); int shim_shmget(key_t key, size_t size, int shmflg); void* shim_shmat(int shmid, const void* shmaddr, int shmflg); int shim_shmctl(int shmid, int cmd, struct shmid_ds* buf); -int shim_dup(int fd); -int shim_dup2(int oldfd, int newfd); +int shim_dup(unsigned int fd); +int shim_dup2(unsigned int oldfd, unsigned int newfd); int shim_pause(void); int shim_nanosleep(const struct __kernel_timespec* rqtp, struct __kernel_timespec* rmtp); int shim_getitimer(int which, struct __kernel_itimerval* value); @@ -806,7 +806,7 @@ int shim_accept4(int sockfd, struct sockaddr* addr, socklen_t* addrlen, int flag int shim_signalfd4(int ufd, __sigset_t* user_mask, size_t sizemask, int flags); int shim_eventfd2(unsigned int count, int flags); int shim_epoll_create1(int flags); -int shim_dup3(int oldfd, int newfd, int flags); +int shim_dup3(unsigned int oldfd, unsigned int newfd, int flags); int shim_pipe2(int* fildes, int flags); int shim_inotify_init1(int flags); int shim_preadv(unsigned long fd, const struct iovec* vec, unsigned long vlen, unsigned long pos_l, diff --git a/LibOS/shim/src/bookkeep/shim_handle.c b/LibOS/shim/src/bookkeep/shim_handle.c index 7bd58b71..b4aa518b 100644 --- a/LibOS/shim/src/bookkeep/shim_handle.c +++ b/LibOS/shim/src/bookkeep/shim_handle.c @@ -127,7 +127,7 @@ PAL_HANDLE shim_stdio = NULL; static int __set_new_fd_handle(struct shim_fd_handle** fdhdl, FDTYPE fd, struct shim_handle* hdl, int flags); -static struct shim_handle_map* __enlarge_handle_map(struct shim_handle_map* map, FDTYPE size); +static int __enlarge_handle_map(struct shim_handle_map* map, size_t size); int init_handle(void) { if (!create_lock(&handle_mgr_lock)) { @@ -141,6 +141,7 @@ int init_handle(void) { } int init_important_handles(void) { + int ret; struct shim_thread* thread = get_cur_thread(); if (thread->handle_map) @@ -159,14 +160,14 @@ int init_important_handles(void) { lock(&handle_map->lock); if (handle_map->fd_size < 3) { - if (!__enlarge_handle_map(handle_map, INIT_HANDLE_MAP_SIZE)) { + ret = __enlarge_handle_map(handle_map, INIT_HANDLE_MAP_SIZE); + if (ret < 0) { unlock(&handle_map->lock); - return -ENOMEM; + return ret; } } struct shim_handle* hdl = NULL; - int ret; for (int fd = 0; fd < 3; fd++) if (!HANDLE_ALLOCATED(handle_map->map[fd])) { @@ -328,8 +329,8 @@ int set_new_fd_handle(struct shim_handle* hdl, int flags, struct shim_handle_map if (fd >= handle_map->fd_size) { // no space left, need to enlarge handle_map->map - if (!__enlarge_handle_map(handle_map, handle_map->fd_size * 2)) { - ret = -ENOMEM; + ret = __enlarge_handle_map(handle_map, handle_map->fd_size * 2); + if (ret < 0) { goto out; } } @@ -355,8 +356,8 @@ out: int set_new_fd_handle_by_fd(FDTYPE fd, struct shim_handle* hdl, int flags, struct shim_handle_map* handle_map) { - int new_size = 0; - int ret = 0; + size_t new_size = 0; + int ret = 0; if (!handle_map && !(handle_map = get_cur_handle_map(NULL))) return -EBADF; @@ -370,12 +371,14 @@ int set_new_fd_handle_by_fd(FDTYPE fd, struct shim_handle* hdl, int flags, goto extend; if (fd >= handle_map->fd_size) { - new_size = handle_map->fd_size < new_size ? new_size : handle_map->fd_size; + if (handle_map->fd_size >= new_size) + new_size = handle_map->fd_size; extend: - while (new_size <= fd) new_size *= 2; + while (new_size <= fd) + new_size *= 2; - if (!__enlarge_handle_map(handle_map, new_size)) { - ret = -ENOMEM; + ret = __enlarge_handle_map(handle_map, new_size); + if (ret < 0) { goto out; } } @@ -569,22 +572,24 @@ static struct shim_handle_map* get_new_handle_map(FDTYPE size) { return handle_map; } -static struct shim_handle_map* __enlarge_handle_map(struct shim_handle_map* map, FDTYPE size) { +static int __enlarge_handle_map(struct shim_handle_map* map, size_t size) { assert(locked(&map->lock)); + if (size > get_rlimit_cur(RLIMIT_NOFILE)) + return -EBADF; + if (size <= map->fd_size) - return map; + return 0; struct shim_fd_handle** new_map = calloc(size, sizeof(new_map[0])); - if (!new_map) - return NULL; + return -ENOMEM; memcpy(new_map, map->map, map->fd_size * sizeof(new_map[0])); free(map->map); map->map = new_map; map->fd_size = size; - return map; + return 0; } int dup_handle_map(struct shim_handle_map** new, struct shim_handle_map* old_map) { diff --git a/LibOS/shim/src/shim_syscalls.c b/LibOS/shim/src/shim_syscalls.c index 302c94b8..8d97900d 100644 --- a/LibOS/shim/src/shim_syscalls.c +++ b/LibOS/shim/src/shim_syscalls.c @@ -224,10 +224,10 @@ SHIM_SYSCALL_PASSTHROUGH(shmat, 3, void*, int, shmid, const void*, shmaddr, int, SHIM_SYSCALL_PASSTHROUGH(shmctl, 3, int, int, shmid, int, cmd, struct shmid_ds*, buf) /* dup: sys/shim_dup.c */ -DEFINE_SHIM_SYSCALL(dup, 1, shim_do_dup, int, int, fd) +DEFINE_SHIM_SYSCALL(dup, 1, shim_do_dup, int, unsigned int, fd) /* dup2: sys/shim_dup.c */ -DEFINE_SHIM_SYSCALL(dup2, 2, shim_do_dup2, int, int, oldfd, int, newfd) +DEFINE_SHIM_SYSCALL(dup2, 2, shim_do_dup2, int, unsigned int, oldfd, unsigned int, newfd) /* pause: sys/shim_sleep.c */ DEFINE_SHIM_SYSCALL(pause, 0, shim_do_pause, int) @@ -989,7 +989,8 @@ DEFINE_SHIM_SYSCALL (eventfd2, 2, shim_do_eventfd2, int, unsigned int, count, in DEFINE_SHIM_SYSCALL(epoll_create1, 1, shim_do_epoll_create1, int, int, flags) /* dup3: sys/shim_dup.c */ -DEFINE_SHIM_SYSCALL(dup3, 3, shim_do_dup3, int, int, oldfd, int, newfd, int, flags) +DEFINE_SHIM_SYSCALL(dup3, 3, shim_do_dup3, int, unsigned int, oldfd, unsigned int, newfd, int, + flags) /* pipe2: sys/shim_pipe.c */ DEFINE_SHIM_SYSCALL(pipe2, 2, shim_do_pipe2, int, int*, fildes, int, flags) diff --git a/LibOS/shim/src/sys/shim_dup.c b/LibOS/shim/src/sys/shim_dup.c index 3b63f660..d660fc18 100644 --- a/LibOS/shim/src/sys/shim_dup.c +++ b/LibOS/shim/src/sys/shim_dup.c @@ -30,9 +30,9 @@ #include #include -int shim_do_dup(int fd) { +int shim_do_dup(unsigned int fd) { struct shim_handle_map* handle_map = get_cur_handle_map(NULL); - int flags = 0; + int flags = 0; struct shim_handle* hdl = get_fd_handle(fd, &flags, handle_map); if (!hdl) @@ -40,12 +40,14 @@ int shim_do_dup(int fd) { int vfd = set_new_fd_handle(hdl, flags, handle_map); put_handle(hdl); - return vfd < 0 ? -EMFILE : vfd; + return vfd == -ENOMEM ? -EMFILE : vfd; } -int shim_do_dup2(int oldfd, int newfd) { - struct shim_handle_map* handle_map = get_cur_handle_map(NULL); +int shim_do_dup2(unsigned int oldfd, unsigned int newfd) { + if (oldfd == newfd) + return -EINVAL; + struct shim_handle_map* handle_map = get_cur_handle_map(NULL); struct shim_handle* hdl = get_fd_handle(oldfd, NULL, handle_map); if (!hdl) return -EBADF; @@ -57,12 +59,15 @@ int shim_do_dup2(int oldfd, int newfd) { int vfd = set_new_fd_handle_by_fd(newfd, hdl, 0, handle_map); put_handle(hdl); - return vfd < 0 ? -EMFILE : vfd; + return vfd == -ENOMEM ? -EMFILE : vfd; } -int shim_do_dup3(int oldfd, int newfd, int flags) { +int shim_do_dup3(unsigned int oldfd, unsigned int newfd, int flags) { + if ((flags & ~O_CLOEXEC) || oldfd == newfd) + return -EINVAL; + struct shim_handle_map* handle_map = get_cur_handle_map(NULL); - struct shim_handle* hdl = get_fd_handle(oldfd, NULL, handle_map); + struct shim_handle* hdl = get_fd_handle(oldfd, NULL, handle_map); if (!hdl) return -EBADF; @@ -73,5 +78,5 @@ int shim_do_dup3(int oldfd, int newfd, int flags) { int vfd = set_new_fd_handle_by_fd(newfd, hdl, flags, handle_map); put_handle(hdl); - return vfd < 0 ? -EMFILE : vfd; + return vfd == -ENOMEM ? -EMFILE : vfd; } diff --git a/LibOS/shim/test/ltp/ltp.cfg b/LibOS/shim/test/ltp/ltp.cfg index a79c18a6..276a1ef5 100644 --- a/LibOS/shim/test/ltp/ltp.cfg +++ b/LibOS/shim/test/ltp/ltp.cfg @@ -248,38 +248,21 @@ skip = yes [dup05] skip = yes -[dup06] -skip = yes - [dup07] must-pass = 1 2 3 -[dup201] -must-pass = - 1 - 2 - [dup202] must-pass = 1 2 3 -[dup204] -timeout = 40 - -[dup205] -skip = yes - [dup3_01] skip = yes -[dup3_02] -skip = yes - # complex test, not all of its checking is implemented in Graphene [epoll01] skip = yes