From e5cc85fdc42b84c6b41da59404eea7b0fc8c42a1 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Sat, 5 Oct 2013 16:31:35 +0200 Subject: [PATCH] Extend dupfrom(2) into copyfd(2) This single function allows copying file descriptors from and to processes, and closing a previously copied remote file descriptor. This function replaces the five FD-related UDS backcalls. While it limits the total number of in-flight file descriptors to OPEN_MAX, this change greatly improves crash recovery support of UDS, since all in-flight file descriptors will be closed instead of keeping them open indefinitely (causing VFS to crash on system shutdown). With the new copyfd call, UDS becomes simpler, and the concept of filps is no longer exposed outside of VFS. This patch also moves the checkperms(2) stub into libminlib, thus fully abstracting away message details of VFS communication from UDS. Change-Id: Idd32ad390a566143c8ef66955e5ae2c221cff966 --- drivers/uds/Makefile | 2 +- drivers/uds/ioc_uds.c | 101 +++++++------- drivers/uds/uds.h | 11 -- drivers/uds/vfs_uds.c | 168 ---------------------- drivers/vnd/NOTES | 19 ++- drivers/vnd/vnd.c | 3 +- etc/system.conf | 4 +- include/lib.h | 3 +- include/minix/callnr.h | 11 +- include/minix/com.h | 22 +-- lib/libminlib/Makefile | 6 +- lib/libminlib/checkperms.c | 27 ++++ lib/libminlib/copyfd.c | 15 ++ lib/libminlib/dupfrom.c | 14 -- servers/pm/table.c | 9 +- servers/vfs/filedes.c | 278 ++++++++----------------------------- servers/vfs/path.c | 8 +- servers/vfs/proto.h | 15 +- servers/vfs/table.c | 9 +- test/tvnd.c | 2 +- 20 files changed, 195 insertions(+), 532 deletions(-) delete mode 100644 drivers/uds/vfs_uds.c create mode 100644 lib/libminlib/checkperms.c create mode 100644 lib/libminlib/copyfd.c delete mode 100644 lib/libminlib/dupfrom.c diff --git a/drivers/uds/Makefile b/drivers/uds/Makefile index 1348ec41b..8f339297d 100644 --- a/drivers/uds/Makefile +++ b/drivers/uds/Makefile @@ -1,6 +1,6 @@ # Makefile for the UNIX Domain Sockets driver (UDS) PROG= uds -SRCS= uds.c ioc_uds.c vfs_uds.c +SRCS= uds.c ioc_uds.c DPADD+= ${LIBCHARDRIVER} ${LIBSYS} LDADD+= -lchardriver -lsys diff --git a/drivers/uds/ioc_uds.c b/drivers/uds/ioc_uds.c index b9336fdfe..066069696 100644 --- a/drivers/uds/ioc_uds.c +++ b/drivers/uds/ioc_uds.c @@ -166,8 +166,9 @@ do_connect(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) sizeof(struct sockaddr_un))) != OK) return rc; - if ((rc = vfs_check_perms(uds_fd_table[minor].owner, &addr)) != OK) - return rc; + if (checkperms(uds_fd_table[minor].owner, addr.sun_path, + UNIX_PATH_MAX) != OK) + return -errno; /* * Look for a socket of the same type that is listening on the @@ -354,8 +355,9 @@ do_bind(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) if (addr.sun_path[0] == '\0') return ENOENT; - if ((rc = vfs_check_perms(uds_fd_table[minor].owner, &addr)) != OK) - return rc; + if (checkperms(uds_fd_table[minor].owner, addr.sun_path, + UNIX_PATH_MAX) != OK) + return -errno; /* Make sure the address isn't already in use by another socket. */ for (i = 0; i < NR_FDS; i++) { @@ -609,8 +611,9 @@ do_sendto(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) if (addr.sun_family != AF_UNIX || addr.sun_path[0] == '\0') return EINVAL; - if ((rc = vfs_check_perms(uds_fd_table[minor].owner, &addr)) != OK) - return rc; + if (checkperms(uds_fd_table[minor].owner, addr.sun_path, + UNIX_PATH_MAX) != OK) + return -errno; memcpy(&uds_fd_table[minor].target, &addr, sizeof(struct sockaddr_un)); @@ -628,17 +631,20 @@ do_recvfrom(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) } static int -msg_control_read(struct msg_control *msg_ctrl, struct ancillary *data, - devminor_t minor, int *new_total) +send_fds(devminor_t minor, struct msg_control *msg_ctrl, + struct ancillary *data) { int i, rc, nfds, totalfds; + endpoint_t from_ep; struct msghdr msghdr; struct cmsghdr *cmsg = NULL; - dprintf(("UDS: msg_control_read(%d)\n", minor)); + dprintf(("UDS: send_fds(%d)\n", minor)); + + from_ep = uds_fd_table[minor].owner; /* Obtain this socket's credentials. */ - if ((rc = getnucred(uds_fd_table[minor].owner, &data->cred)) < 0) + if ((rc = getnucred(from_ep, &data->cred)) < 0) return -errno; dprintf(("UDS: minor=%d cred={%d,%d,%d}\n", minor, data->cred.pid, @@ -669,41 +675,31 @@ msg_control_read(struct msg_control *msg_ctrl, struct ancillary *data, } } - *new_total = totalfds; - - return OK; -} - -static int -send_fds(devminor_t minor, struct ancillary *data, int new_total) -{ - int rc, i, j; + for (i = data->nfiledes; i < totalfds; i++) { + if ((rc = copyfd(from_ep, data->fds[i], COPYFD_FROM)) < 0) { + rc = -errno; - dprintf(("UDS: send_fds(%d)\n", minor)); + printf("UDS: copyfd(COPYFD_FROM) failed: %d\n", rc); - /* Verify the file descriptors and get their filps. */ - for (i = data->nfiledes; i < new_total; i++) { - if ((rc = vfs_verify_fd(uds_fd_table[minor].owner, - data->fds[i], &data->filps[i])) != OK) - return rc; - } + /* Revert the successful copyfd() calls made so far. */ + for (i--; i >= data->nfiledes; i--) + close(data->fds[i]); - /* Set them as in-flight. */ - for (i = data->nfiledes; i < new_total; i++) { - if ((rc = vfs_set_filp(data->filps[i])) != OK) { - for (j = i - 1; j >= data->nfiledes; j--) - vfs_put_filp(data->filps[j]); /* revert */ return rc; } + + dprintf(("UDS: send_fds(): %d -> %d\n", data->fds[i], rc)); + + data->fds[i] = rc; /* this is now the local FD */ } - data->nfiledes = new_total; + data->nfiledes = totalfds; return OK; } /* - * This function calls put_filp() for all of the FDs in data. This is used + * This function calls close() for all of the FDs in flight. This is used * when a Unix Domain Socket is closed and there exists references to file * descriptors that haven't been received with recvmsg(). */ @@ -717,10 +713,9 @@ uds_clear_fds(devminor_t minor, struct ancillary *data) for (i = 0; i < data->nfiledes; i++) { dprintf(("UDS: uds_clear_fds() => %d\n", data->fds[i])); - vfs_put_filp(data->filps[i]); + close(data->fds[i]); data->fds[i] = -1; - data->filps[i] = NULL; } data->nfiledes = 0; @@ -732,7 +727,7 @@ static int recv_fds(devminor_t minor, struct ancillary *data, struct msg_control *msg_ctrl) { - int rc, i, j; + int rc, i, j, fds[OPEN_MAX]; struct msghdr msghdr; struct cmsghdr *cmsg; endpoint_t to_ep; @@ -751,28 +746,30 @@ recv_fds(devminor_t minor, struct ancillary *data, /* Copy to the target endpoint. */ for (i = 0; i < data->nfiledes; i++) { - if ((rc = vfs_copy_filp(to_ep, data->filps[i])) < 0) { - /* Revert vfs_set_filp() calls. */ - for (j = 0; j < data->nfiledes; j++) - vfs_put_filp(data->filps[j]); + if ((rc = copyfd(to_ep, data->fds[i], COPYFD_TO)) < 0) { + rc = -errno; + + printf("UDS: copyfd(COPYFD_TO) failed: %d\n", rc); - /* Revert vfs_copy_filp() calls. */ - for (j = i - 1; j >= 0; j--) - vfs_cancel_fd(to_ep, data->fds[j]); + /* Revert the successful copyfd() calls made so far. */ + for (i--; i >= 0; i--) + (void) copyfd(to_ep, fds[i], COPYFD_CLOSE); return rc; } - data->fds[i] = rc; /* data->fds[i] now has the new FD */ + + fds[i] = rc; /* this is now the remote FD */ } + /* Close the local copies only once the entire procedure succeeded. */ for (i = 0; i < data->nfiledes; i++) { - vfs_put_filp(data->filps[i]); + dprintf(("UDS: recv_fds(): %d -> %d\n", data->fds[i], fds[i])); - dprintf(("UDS: recv_fds() => %d\n", data->fds[i])); + ((int *)CMSG_DATA(cmsg))[i] = fds[i]; + + close(data->fds[i]); - ((int *)CMSG_DATA(cmsg))[i] = data->fds[i]; data->fds[i] = -1; - data->filps[i] = NULL; } data->nfiledes = 0; @@ -807,7 +804,7 @@ recv_cred(devminor_t minor, struct ancillary *data, static int do_sendmsg(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) { - int peer, rc, i, new_total; + int peer, rc, i; struct msg_control msg_ctrl; dprintf(("UDS: do_sendmsg(%d)\n", minor)); @@ -855,11 +852,7 @@ do_sendmsg(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant) * The receiver will get the current file descriptors plus the new * ones. */ - if ((rc = msg_control_read(&msg_ctrl, - &uds_fd_table[peer].ancillary_data, minor, &new_total)) != OK) - return rc; - - return send_fds(minor, &uds_fd_table[peer].ancillary_data, new_total); + return send_fds(minor, &msg_ctrl, &uds_fd_table[peer].ancillary_data); } static int diff --git a/drivers/uds/uds.h b/drivers/uds/uds.h index 24fa88f97..150d8b06f 100644 --- a/drivers/uds/uds.h +++ b/drivers/uds/uds.h @@ -28,11 +28,8 @@ #define dprintf(x) #endif -typedef void* filp_id_t; - /* ancillary data to be sent */ struct ancillary { - filp_id_t filps[OPEN_MAX]; int fds[OPEN_MAX]; int nfiledes; struct uucred cred; @@ -201,12 +198,4 @@ ssize_t uds_perform_read(devminor_t minor, endpoint_t endpt, cp_grant_id_t grant, size_t size, int pretend); void uds_unsuspend(devminor_t minor); -/* vfs_uds.c */ -int vfs_check_perms(endpoint_t ep, struct sockaddr_un *addr); -int vfs_verify_fd(endpoint_t ep, int fd, filp_id_t *filp); -int vfs_set_filp(filp_id_t sfilp); -int vfs_copy_filp(endpoint_t to_ep, filp_id_t cfilp); -int vfs_put_filp(filp_id_t pfilp); -int vfs_cancel_fd(endpoint_t ep, int fd); - #endif /* !__UDS_UDS_H */ diff --git a/drivers/uds/vfs_uds.c b/drivers/uds/vfs_uds.c deleted file mode 100644 index 7c8748ea2..000000000 --- a/drivers/uds/vfs_uds.c +++ /dev/null @@ -1,168 +0,0 @@ -/* - * Unix Domain Sockets Implementation (PF_UNIX, PF_LOCAL) - * This code provides communication stubs for backcalls to VFS. - */ - -#include "uds.h" - -/* - * Check the permissions of a socket file. - */ -int -vfs_check_perms(endpoint_t ep, struct sockaddr_un *addr) -{ - int rc; - message m; - cp_grant_id_t grant_id; - - dprintf(("UDS: vfs_check_perms(%d)\n", ep)); - - grant_id = cpf_grant_direct(VFS_PROC_NR, (vir_bytes) addr->sun_path, - UNIX_PATH_MAX, CPF_READ | CPF_WRITE); - - /* Ask VFS to verify the permissions. */ - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_CHECK_PERMS; - m.VFS_UDS_ENDPT = ep; - m.VFS_UDS_GRANT = grant_id; - m.VFS_UDS_COUNT = UNIX_PATH_MAX; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - cpf_revoke(grant_id); - - dprintf(("UDS: VFS reply => %d, \"%s\"\n", m.m_type, - addr->sun_path)); - - return m.m_type; /* reply code: OK, ELOOP, etc. */ -} - -/* - * Verify whether the given file descriptor is valid for the given process, and - * obtain a filp object identifier upon success. - */ -int -vfs_verify_fd(endpoint_t ep, int fd, filp_id_t *filp) -{ - int rc; - message m; - - dprintf(("UDS: vfs_verify_fd(%d, %d)\n", ep, fd)); - - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_VERIFY_FD; - m.VFS_UDS_ENDPT = ep; - m.VFS_UDS_FD = fd; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - dprintf(("UDS: VFS reply => %d, %p\n", m.m_type, m.VFS_UDS_FILP)); - - if (m.m_type != OK) - return m.m_type; - - *filp = m.VFS_UDS_FILP; - return OK; -} - -/* - * Mark a filp object as in flight, that is, in use by UDS. - */ -int -vfs_set_filp(filp_id_t sfilp) -{ - int rc; - message m; - - dprintf(("UDS: set_filp(%p)\n", sfilp)); - - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_SET_FILP; - m.VFS_UDS_FILP = sfilp; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - dprintf(("UDS: VFS reply => %d\n", m.m_type)); - - return m.m_type; /* reply code: OK, ELOOP, etc. */ -} - -/* - * Copy a filp object into a process, yielding a file descriptor. - */ -int -vfs_copy_filp(endpoint_t to_ep, filp_id_t cfilp) -{ - int rc; - message m; - - dprintf(("UDS: vfs_copy_filp(%d, %p)\n", to_ep, cfilp)); - - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_COPY_FILP; - m.VFS_UDS_ENDPT = to_ep; - m.VFS_UDS_FILP = cfilp; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - dprintf(("UDS: VFS reply => %d\n", m.m_type)); - - return m.m_type; -} - -/* - * Mark a filp object as no longer in flight. - */ -int -vfs_put_filp(filp_id_t pfilp) -{ - int rc; - message m; - - dprintf(("UDS: vfs_put_filp(%p)\n", pfilp)); - - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_PUT_FILP; - m.VFS_UDS_FILP = pfilp; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - dprintf(("UDS: VFS reply => %d\n", m.m_type)); - - return m.m_type; /* reply code: OK, ELOOP, etc. */ -} - -/* - * Undo creation of a file descriptor in a process. - */ -int -vfs_cancel_fd(endpoint_t ep, int fd) -{ - int rc; - message m; - - dprintf(("UDS: vfs_cancel_fd(%d,%d)\n", ep, fd)); - - memset(&m, '\0', sizeof(message)); - - m.m_type = VFS_UDS_CANCEL_FD; - m.VFS_UDS_ENDPT = ep; - m.VFS_UDS_FD = fd; - - if ((rc = sendrec(VFS_PROC_NR, &m)) != OK) - panic("error sending to VFS: %d\n", rc); - - dprintf(("UDS: VFS reply => %d\n", m.m_type)); - - return m.m_type; /* reply code: OK, ELOOP, etc. */ -} diff --git a/drivers/vnd/NOTES b/drivers/vnd/NOTES index 382ead657..69d0f83aa 100644 --- a/drivers/vnd/NOTES +++ b/drivers/vnd/NOTES @@ -23,7 +23,7 @@ VFS threads. Of course, this comes at the cost of having more VND driver processes; in order to avoid this cost in the common case, driver instances are dynamically started and stopped by vndconfig(8). -dupfrom(2) instead of openas(2): Compared to the NetBSD interface, the MINIX3 +copyfd(2) instead of openas(2): Compared to the NetBSD interface, the MINIX3 VND API requires that the user program configuring a device pass in a file descriptor in the vnd_ioctl structure instead of a pointer to a path name. While binary compatibility with NetBSD would be impossible anyway (MINIX3 can @@ -62,24 +62,29 @@ MINIX3 userland happy. FUTURE IMPROVEMENTS Currently, the VND driver instances are run as root just and only because the -dupfrom(2) call requires root. Obviously, nonroot user processes should never +copyfd(2) call requires root. Obviously, nonroot user processes should never be able to copy file descriptors from arbitrary processes, and thus, some security check is required there. However, an access control list for VFS calls would be a much better solution: in that case, VND driver processes can be -given exclusive rights to the use of the dupfrom(2) call, while they can be +given exclusive rights to the use of the copyfd(2) call, while they can be given a normal driver UID at the same time. In MINIX3's dependability model, drivers are generally not considered to be malicious. However, the VND case is interesting because it is possible to isolate individual driver instances to the point of actual "least authority". -The dupfrom(2) call currently allows any file descriptor to be copied, but it +The copyfd(2) call currently allows any file descriptor to be copied, but it would be possible to extend the scheme to let user processes (and vndconfig(8) -in particular) mark the file descriptors that may be the target of a dupfrom(2) +in particular) mark the file descriptors that may be the target of a copyfd(2) call. One of several schemes may be implemented in VFS for this purpose. For example, each process could be allowed to mark one of its file descriptors as -"copyable" using a new VFS call, and VFS would then allow dupfrom(2) only on a +"copyable" using a new VFS call, and VFS would then allow copyfd(2) only on a "copyable" file descriptor from a process blocked on a call to the driver that -invoked dupfrom(2). This approach precludes hiding a VND driver behind a RAID +invoked copyfd(2). This approach precludes hiding a VND driver behind a RAID or FBD (etc) driver, but more sophisticated approaches can solve that as well. Regardless of the scheme, the end result would be a situation where the VND drivers are strictly limited to operating on the resources given to them. + +Note that copyfd(2) was originally called dupfrom(2), and then extended to copy +file descriptors *to* remote processes as well. The latter is not as security +sensitive, but may have to be restricted in a similar way. If this is not +possible, copyfd(2) can always be split into multiple calls. diff --git a/drivers/vnd/vnd.c b/drivers/vnd/vnd.c index 422ce0088..7289bc184 100644 --- a/drivers/vnd/vnd.c +++ b/drivers/vnd/vnd.c @@ -384,7 +384,8 @@ vnd_ioctl(devminor_t UNUSED(minor), unsigned long request, endpoint_t endpt, * making the IOCTL call. The result is either a newly * allocated file descriptor or an error. */ - if ((state.fd = dupfrom(user_endpt, vnd.vnd_fildes)) == -1) + if ((state.fd = copyfd(user_endpt, vnd.vnd_fildes, + COPYFD_FROM)) == -1) return -errno; /* The target file must be regular. */ diff --git a/etc/system.conf b/etc/system.conf index dc6617082..841c32c51 100644 --- a/etc/system.conf +++ b/etc/system.conf @@ -698,7 +698,7 @@ service vnd ipc SYSTEM vfs rs vm ; - uid 0; # only for dupfrom(2) + uid 0; # only for copyfd(2) }; service uds @@ -706,5 +706,5 @@ service uds ipc SYSTEM vfs rs vm ; - uid 0; # for various VFS backcalls, until we have ACLs + uid 0; # only for checkperms(2) and copyfd(2) }; diff --git a/include/lib.h b/include/lib.h index 92cf2a5fb..006cbb078 100644 --- a/include/lib.h +++ b/include/lib.h @@ -44,7 +44,8 @@ int mapdriver(char *label, int major, int style, int flags); pid_t getnpid(endpoint_t proc_ep); uid_t getnuid(endpoint_t proc_ep); gid_t getngid(endpoint_t proc_ep); -int dupfrom(endpoint_t endpt, int fd); +int checkperms(endpoint_t endpt, char *path, size_t size); +int copyfd(endpoint_t endpt, int fd, int what); ssize_t pread64(int fd, void *buf, size_t count, u64_t where); ssize_t pwrite64(int fd, const void *buf, size_t count, u64_t where); diff --git a/include/minix/callnr.h b/include/minix/callnr.h index 785fc3781..5834324f7 100644 --- a/include/minix/callnr.h +++ b/include/minix/callnr.h @@ -1,4 +1,4 @@ -#define NCALLS 130 /* number of system calls allowed */ +#define NCALLS 125 /* number of system calls allowed */ /* In case it isn't obvious enough: this list is sorted numerically. */ #define EXIT 1 @@ -47,7 +47,7 @@ #define FSTAT 52 #define LSTAT 53 #define IOCTL 54 -#define DUPFROM 56 +#define COPYFD 56 #define FS_READY 57 #define PIPE2 58 #define EXEC 59 @@ -112,9 +112,4 @@ #define MAPDRIVER 122 /* to VFS, map a device */ #define GETRUSAGE 123 /* to PM, VFS */ -#define VFS_UDS_CHECK_PERMS 124 /* to VFS */ -#define VFS_UDS_VERIFY_FD 125 /* to VFS */ -#define VFS_UDS_SET_FILP 126 /* to VFS */ -#define VFS_UDS_COPY_FILP 127 /* to VFS */ -#define VFS_UDS_PUT_FILP 128 /* to VFS */ -#define VFS_UDS_CANCEL_FD 129 /* to VFS */ +#define VFS_CHECKPERMS 124 /* to VFS */ diff --git a/include/minix/com.h b/include/minix/com.h index 4647506f3..72793dd63 100644 --- a/include/minix/com.h +++ b/include/minix/com.h @@ -882,16 +882,18 @@ #define VFS_IOCTL_REQ m2_i3 #define VFS_IOCTL_ARG m2_p1 -/* Field names for the UDS backcalls to VFS. */ -#define VFS_UDS_ENDPT m2_i1 -#define VFS_UDS_GRANT m2_i2 -#define VFS_UDS_COUNT m2_i3 -#define VFS_UDS_FD m2_i3 -#define VFS_UDS_FILP m2_p1 - -/* Field names for the dupfrom(2) call. */ -#define VFS_DUPFROM_ENDPT m1_i1 -#define VFS_DUPFROM_FD m1_i2 +/* Field names for the checkperms(2) call. */ +#define VFS_CHECKPERMS_ENDPT m2_i1 +#define VFS_CHECKPERMS_GRANT m2_i2 +#define VFS_CHECKPERMS_COUNT m2_i3 + +/* Field names for the copyfd(2) call. */ +#define VFS_COPYFD_ENDPT m1_i1 +#define VFS_COPYFD_FD m1_i2 +#define VFS_COPYFD_WHAT m1_i3 +# define COPYFD_FROM 0 /* copy file descriptor from remote process */ +# define COPYFD_TO 1 /* copy file descriptor to remote process */ +# define COPYFD_CLOSE 2 /* close file descriptor in remote process */ /* Field names for GETRUSAGE related calls */ #define RU_ENDPT m1_i1 /* indicates a process for sys_getrusage */ diff --git a/lib/libminlib/Makefile b/lib/libminlib/Makefile index 5d5092d52..cc288f157 100644 --- a/lib/libminlib/Makefile +++ b/lib/libminlib/Makefile @@ -31,9 +31,9 @@ SRCS+= servxcheck.c SRCS+= paramvalue.c # Minix servers/drivers syscall. FIXME: these should be moved into libsys. -SRCS+= dupfrom.c getngid.c getnpid.c getnprocnr.c getnucred.c getnuid.c \ - getprocnr.c mapdriver.c vm_memctl.c vm_set_priv.c vm_query_exit.c \ - vm_update.c +SRCS+= checkperms.c copyfd.c getngid.c getnpid.c getnprocnr.c getnucred.c \ + getnuid.c getprocnr.c mapdriver.c vm_memctl.c vm_set_priv.c \ + vm_query_exit.c vm_update.c SRCS+= oneC_sum.c diff --git a/lib/libminlib/checkperms.c b/lib/libminlib/checkperms.c new file mode 100644 index 000000000..154f48734 --- /dev/null +++ b/lib/libminlib/checkperms.c @@ -0,0 +1,27 @@ +#include +#include +#include +#include + +int +checkperms(endpoint_t endpt, char *path, size_t size) +{ + cp_grant_id_t grant; + message m; + int r; + + if ((grant = cpf_grant_direct(VFS_PROC_NR, (vir_bytes) path, size, + CPF_READ | CPF_WRITE)) == GRANT_INVALID) + return -1; /* called function sets errno */ + + memset(&m, 0, sizeof(m)); + m.VFS_CHECKPERMS_ENDPT = endpt; + m.VFS_CHECKPERMS_GRANT = grant; + m.VFS_CHECKPERMS_COUNT = size; + + r = _syscall(VFS_PROC_NR, VFS_CHECKPERMS, &m); + + cpf_revoke(grant); /* does not touch errno */ + + return r; +} diff --git a/lib/libminlib/copyfd.c b/lib/libminlib/copyfd.c new file mode 100644 index 000000000..7f5ed71c5 --- /dev/null +++ b/lib/libminlib/copyfd.c @@ -0,0 +1,15 @@ +#include +#include + +int +copyfd(endpoint_t endpt, int fd, int what) +{ + message m; + + memset(&m, 0, sizeof(m)); + m.VFS_COPYFD_ENDPT = endpt; + m.VFS_COPYFD_FD = fd; + m.VFS_COPYFD_WHAT = what; + + return _syscall(VFS_PROC_NR, COPYFD, &m); +} diff --git a/lib/libminlib/dupfrom.c b/lib/libminlib/dupfrom.c deleted file mode 100644 index b244fbbef..000000000 --- a/lib/libminlib/dupfrom.c +++ /dev/null @@ -1,14 +0,0 @@ -#include -#include - -int -dupfrom(endpoint_t endpt, int fd) -{ - message m; - - memset(&m, 0, sizeof(m)); - m.VFS_DUPFROM_ENDPT = endpt; - m.VFS_DUPFROM_FD = fd; - - return _syscall(VFS_PROC_NR, DUPFROM, &m); -} diff --git a/servers/pm/table.c b/servers/pm/table.c index 2150f0bf6..e7de0df50 100644 --- a/servers/pm/table.c +++ b/servers/pm/table.c @@ -67,7 +67,7 @@ int (*call_vec[])(void) = { no_sys, /* 53 = (lstat) */ no_sys, /* 54 = ioctl */ no_sys, /* 55 = fcntl */ - no_sys, /* 56 = dupfrom */ + no_sys, /* 56 = copyfd */ no_sys, /* 57 = unused */ no_sys, /* 58 = unused */ do_exec, /* 59 = execve */ @@ -135,12 +135,7 @@ int (*call_vec[])(void) = { no_sys, /* 121 = (task reply) */ no_sys, /* 122 = (map driver ) */ do_getrusage, /* 123 = getrusage */ - no_sys, /* 124 = (check_perms) */ - no_sys, /* 125 = (verify_fd) */ - no_sys, /* 126 = (set_filp) */ - no_sys, /* 127 = (copy_filp) */ - no_sys, /* 128 = (put_filp) */ - no_sys, /* 129 = (cancel_fd) */ + no_sys, /* 124 = (checkperms) */ }; /* This should not fail with "array size is negative": */ extern int dummy[sizeof(call_vec) == NCALLS * sizeof(call_vec[0]) ? 1 : -1]; diff --git a/servers/vfs/filedes.c b/servers/vfs/filedes.c index 4387b209b..12801bbde 100644 --- a/servers/vfs/filedes.c +++ b/servers/vfs/filedes.c @@ -6,14 +6,7 @@ * find_filp: find a filp slot that points to a given vnode * inval_filp: invalidate a filp and associated fd's, only let close() * happen on it - * do_verify_fd: verify whether the given file descriptor is valid for - * the given endpoint. - * do_set_filp: marks a filp as in-flight. - * do_copy_filp: copies a filp to another endpoint. - * do_put_filp: marks a filp as not in-flight anymore. - * do_cancel_fd: cancel the transaction when something goes wrong for - * the receiver. - * do_dupfrom: copies a filp from another endpoint. + * do_copyfd: copies a file descriptor from or to another endpoint */ #include @@ -26,8 +19,6 @@ #include "vnode.h" -static filp_id_t verify_fd(endpoint_t ep, int fd); - #if LOCK_DEBUG /*===========================================================================* * check_filp_locks * @@ -349,203 +340,6 @@ struct filp *filp2; panic("unable to release filp lock on filp1"); } -/*===========================================================================* - * verify_fd * - *===========================================================================*/ -static filp_id_t verify_fd(ep, fd) -endpoint_t ep; -int fd; -{ -/* Verify whether the file descriptor 'fd' is valid for the endpoint 'ep'. When - * the file descriptor is valid, verify_fd returns a pointer to that filp, else - * it returns NULL. - */ - int slot; - struct filp *rfilp; - - if (isokendpt(ep, &slot) != OK) - return(NULL); - - rfilp = get_filp2(&fproc[slot], fd, VNODE_READ); - - return(rfilp); -} - -/*===========================================================================* - * do_verify_fd * - *===========================================================================*/ -int do_verify_fd(message *m_out) -{ - struct filp *rfilp; - endpoint_t proc_e; - int fd; - - /* This should be replaced with an ACL check. */ - if (!super_user) return EPERM; - - proc_e = job_m_in.VFS_UDS_ENDPT; - fd = job_m_in.VFS_UDS_FD; - - rfilp = (struct filp *) verify_fd(proc_e, fd); - m_out->VFS_UDS_FILP = (void *) rfilp; - if (rfilp != NULL) unlock_filp(rfilp); - return (rfilp != NULL) ? OK : EINVAL; -} - -/*===========================================================================* - * set_filp * - *===========================================================================*/ -int set_filp(sfilp) -filp_id_t sfilp; -{ - if (sfilp == NULL) return(EINVAL); - - lock_filp(sfilp, VNODE_READ); - sfilp->filp_count++; - unlock_filp(sfilp); - - return(OK); -} - -/*===========================================================================* - * do_set_filp * - *===========================================================================*/ -int do_set_filp(message *UNUSED(m_out)) -{ - filp_id_t f; - - /* This should be replaced with an ACL check. */ - if (!super_user) return EPERM; - - f = (filp_id_t) job_m_in.VFS_UDS_FILP; - return set_filp(f); -} - -/*===========================================================================* - * copy_filp * - *===========================================================================*/ -int copy_filp(to_ep, cfilp) -endpoint_t to_ep; -filp_id_t cfilp; -{ - int fd; - int slot; - struct fproc *rfp; - - if (isokendpt(to_ep, &slot) != OK) return(EINVAL); - rfp = &fproc[slot]; - - /* Find an open slot in fp_filp */ - for (fd = 0; fd < OPEN_MAX; fd++) { - if (rfp->fp_filp[fd] == NULL) { - /* Found a free slot, add descriptor */ - rfp->fp_filp[fd] = cfilp; - rfp->fp_filp[fd]->filp_count++; - return(fd); - } - } - - /* File descriptor table is full */ - return(EMFILE); -} - -/*===========================================================================* - * do_copy_filp * - *===========================================================================*/ -int do_copy_filp(message *UNUSED(m_out)) -{ - endpoint_t proc_e; - filp_id_t f; - - /* This should be replaced with an ACL check. */ - if (!super_user) return EPERM; - - proc_e = job_m_in.VFS_UDS_ENDPT; - f = (filp_id_t) job_m_in.VFS_UDS_FILP; - - return copy_filp(proc_e, f); -} - -/*===========================================================================* - * put_filp * - *===========================================================================*/ -int put_filp(pfilp) -filp_id_t pfilp; -{ - if (pfilp == NULL) { - return EINVAL; - } else { - lock_filp(pfilp, VNODE_OPCL); - close_filp(pfilp); - return(OK); - } -} - -/*===========================================================================* - * do_put_filp * - *===========================================================================*/ -int do_put_filp(message *UNUSED(m_out)) -{ - filp_id_t f; - - /* This should be replaced with an ACL check. */ - if (!super_user) return EPERM; - - f = (filp_id_t) job_m_in.VFS_UDS_FILP; - return put_filp(f); -} - -/*===========================================================================* - * cancel_fd * - *===========================================================================*/ -int cancel_fd(ep, fd) -endpoint_t ep; -int fd; -{ - int slot; - struct fproc *rfp; - struct filp *rfilp; - - if (isokendpt(ep, &slot) != OK) return(EINVAL); - rfp = &fproc[slot]; - - /* Check that the input 'fd' is valid */ - rfilp = (struct filp *) verify_fd(ep, fd); - if (rfilp != NULL) { - /* Found a valid descriptor, remove it */ - if (rfp->fp_filp[fd]->filp_count == 0) { - unlock_filp(rfilp); - printf("VFS: filp_count for slot %d fd %d already zero", slot, - fd); - return(EINVAL); - } - rfp->fp_filp[fd]->filp_count--; - rfp->fp_filp[fd] = NULL; - unlock_filp(rfilp); - return(fd); - } - - /* File descriptor is not valid for the endpoint. */ - return(EINVAL); -} - -/*===========================================================================* - * do_cancel_fd * - *===========================================================================*/ -int do_cancel_fd(message *UNUSED(m_out)) -{ - endpoint_t proc_e; - int fd; - - /* This should be replaced with an ACL check. */ - if (!super_user) return EPERM; - - proc_e = job_m_in.VFS_UDS_ENDPT; - fd = job_m_in.VFS_UDS_FD; - - return cancel_fd(proc_e, fd); -} - /*===========================================================================* * close_filp * *===========================================================================*/ @@ -618,33 +412,41 @@ struct filp *f; } /*===========================================================================* - * do_dupfrom * + * do_copyfd * *===========================================================================*/ -int do_dupfrom(message *UNUSED(m_out)) +int do_copyfd(message *UNUSED(m_out)) { -/* Duplicate a file descriptor from another process into the calling process. - * The other process is identified by a magic grant created for it to make the - * initial (IOCTL) request to the calling process. This call has been added - * specifically for the VND driver. +/* Copy a file descriptor between processes, or close a remote file descriptor. + * This call is used as back-call by device drivers (UDS, VND), and is expected + * to be used in response to an IOCTL to such device drivers. */ struct fproc *rfp; struct filp *rfilp; endpoint_t endpt; - int r, fd, slot; + int r, fd, what, slot; /* This should be replaced with an ACL check. */ if (!super_user) return(EPERM); - endpt = (endpoint_t) job_m_in.VFS_DUPFROM_ENDPT; - fd = job_m_in.VFS_DUPFROM_FD; + endpt = (endpoint_t) job_m_in.VFS_COPYFD_ENDPT; + fd = job_m_in.VFS_COPYFD_FD; + what = job_m_in.VFS_COPYFD_WHAT; if (isokendpt(endpt, &slot) != OK) return(EINVAL); rfp = &fproc[slot]; - /* Obtain the filp, but do not lock it yet: we first need to make sure that + /* FIXME: we should now check that the user process is indeed blocked on an + * IOCTL call, so that we can safely mess with its file descriptors. We + * currently do not have the necessary state to verify this, so we assume + * that the call is always used in the right way. + */ + + /* Depending on the operation, get the file descriptor from the caller or the + * user process. Do not lock the filp yet: we first need to make sure that * locking it will not result in a deadlock. */ - if ((rfilp = get_filp2(rfp, fd, VNODE_NONE)) == NULL) + rfilp = get_filp2((what == COPYFD_TO) ? fp : rfp, fd, VNODE_NONE); + if (rfilp == NULL) return(err_code); /* If the filp is involved in an IOCTL by the user process, locking the filp @@ -657,10 +459,46 @@ int do_dupfrom(message *UNUSED(m_out)) if (rfilp->filp_ioctl_fp == rfp) return(EBADF); - /* Now we can safely lock the filp, copy it, and unlock it again. */ + /* Now we can safely lock the filp, copy or close it, and unlock it again. */ lock_filp(rfilp, VNODE_READ); - r = copy_filp(who_e, (filp_id_t) rfilp); + switch (what) { + case COPYFD_FROM: + rfp = fp; + + /* FALLTHROUGH */ + case COPYFD_TO: + /* Find a free file descriptor slot in the local or remote process. */ + for (fd = 0; fd < OPEN_MAX; fd++) + if (rfp->fp_filp[fd] == NULL) + break; + + /* If found, fill the slot and return the slot number. */ + if (fd < OPEN_MAX) { + rfp->fp_filp[fd] = rfilp; + rfilp->filp_count++; + r = fd; + } else + r = EMFILE; + + break; + + case COPYFD_CLOSE: + /* This should be used ONLY to revert a successful copy-to operation, + * and assumes that the filp is still in use by the caller as well. + */ + if (rfilp->filp_count > 1) { + rfilp->filp_count--; + rfp->fp_filp[fd] = NULL; + r = OK; + } else + r = EBADF; + + break; + + default: + r = EINVAL; + } unlock_filp(rfilp); diff --git a/servers/vfs/path.c b/servers/vfs/path.c index eb2e423b6..a2e52ff20 100644 --- a/servers/vfs/path.c +++ b/servers/vfs/path.c @@ -865,13 +865,13 @@ size_t pathlen; } /*===========================================================================* - * do_check_perms * + * do_checkperms * *===========================================================================*/ -int do_check_perms(message *UNUSED(m_out)) +int do_checkperms(message *UNUSED(m_out)) { /* This should be replaced by an ACL check. */ if (!super_user) return EPERM; - return check_perms(job_m_in.VFS_UDS_ENDPT, job_m_in.VFS_UDS_GRANT, - (size_t) job_m_in.VFS_UDS_COUNT); + return check_perms(job_m_in.VFS_CHECKPERMS_ENDPT, + job_m_in.VFS_CHECKPERMS_GRANT, (size_t) job_m_in.VFS_CHECKPERMS_COUNT); } diff --git a/servers/vfs/proto.h b/servers/vfs/proto.h index 0197dc22e..72a0a94f7 100644 --- a/servers/vfs/proto.h +++ b/servers/vfs/proto.h @@ -20,8 +20,6 @@ struct lookup; struct worker_thread; struct job; -typedef struct filp * filp_id_t; - /* comm.c */ int drv_sendrec(endpoint_t drv_e, message *reqm); void fs_cancel(struct vmnt *vmp); @@ -91,17 +89,8 @@ void unlock_filps(struct filp *filp1, struct filp *filp2); void invalidate_filp(struct filp *); void invalidate_filp_by_endpt(endpoint_t proc_e); void invalidate_filp_by_char_major(int major); -int do_verify_fd(message *m_out); -int set_filp(filp_id_t sfilp); -int do_set_filp(message *m_out); -int copy_filp(endpoint_t to_ep, filp_id_t cfilp); -int do_copy_filp(message *m_out); -int put_filp(filp_id_t pfilp); -int do_put_filp(message *m_out); -int cancel_fd(endpoint_t ep, int fd); -int do_cancel_fd(message *m_out); void close_filp(struct filp *fp); -int do_dupfrom(message *m_out); +int do_copyfd(message *m_out); /* fscall.c */ void nested_fs_call(message *m); @@ -182,7 +171,7 @@ void lookup_init(struct lookup *resolve, char *path, int flags, struct vmnt **vmp, struct vnode **vp); int get_name(struct vnode *dirp, struct vnode *entry, char *_name); int canonical_path(char *orig_path, struct fproc *rfp); -int do_check_perms(message *m_out); +int do_checkperms(message *m_out); /* pipe.c */ int do_pipe(message *m_out); diff --git a/servers/vfs/table.c b/servers/vfs/table.c index e5535cf4b..dac713876 100644 --- a/servers/vfs/table.c +++ b/servers/vfs/table.c @@ -70,7 +70,7 @@ int (*call_vec[])(message *m_out) = { do_lstat, /* 53 = lstat */ do_ioctl, /* 54 = ioctl */ do_fcntl, /* 55 = fcntl */ - do_dupfrom, /* 56 = dupfrom */ + do_copyfd, /* 56 = copyfd */ do_fsready, /* 57 = FS proc ready */ do_pipe2, /* 58 = pipe2 */ no_sys, /* 59 = (execve)*/ @@ -138,12 +138,7 @@ int (*call_vec[])(message *m_out) = { no_sys, /* 121 = (task reply) */ do_mapdriver, /* 122 = mapdriver */ do_getrusage, /* 123 = getrusage */ - do_check_perms, /* 124 = from UDS: check_perms */ - do_verify_fd, /* 125 = from UDS: verify_fd */ - do_set_filp, /* 126 = from UDS: set_filp */ - do_copy_filp, /* 127 = from UDS: copy_filp */ - do_put_filp, /* 128 = from UDS: put_filp */ - do_cancel_fd, /* 129 = from UDS: cancel_fd */ + do_checkperms, /* 124 = checkperms */ }; /* This should not fail with "array size is negative": */ extern int dummy[sizeof(call_vec) == NCALLS * sizeof(call_vec[0]) ? 1 : -1]; diff --git a/test/tvnd.c b/test/tvnd.c index 0f6c83331..7ac318778 100644 --- a/test/tvnd.c +++ b/test/tvnd.c @@ -10,7 +10,7 @@ * For now, just test the most dreaded case: the driver being told to use the * file descriptor used to configure the device. Without appropriate checks, * this would cause a deadlock in VFS, since the corresponding filp object is - * locked to perform the ioctl(2) call when VFS gets the dupfrom(2) back-call. + * locked to perform the ioctl(2) call when VFS gets the copyfd(2) back-call. */ int main(int argc, char **argv) -- 2.44.0