From owner-p4-projects@FreeBSD.ORG Mon Jun 15 10:13:22 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 5978A1065672; Mon, 15 Jun 2009 10:13:22 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 193AC106566C for ; Mon, 15 Jun 2009 10:13:22 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 03FFC8FC17 for ; Mon, 15 Jun 2009 10:13:22 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n5FADLBd060558 for ; Mon, 15 Jun 2009 10:13:21 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n5FADLkD060556 for perforce@freebsd.org; Mon, 15 Jun 2009 10:13:21 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Mon, 15 Jun 2009 10:13:21 GMT Message-Id: <200906151013.n5FADLkD060556@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Cc: Subject: PERFORCE change 164410 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jun 2009 10:13:23 -0000 http://perforce.freebsd.org/chv.cgi?CH=164410 Change 164410 by rwatson@rwatson_freebsd_capabilities on 2009/06/15 10:12:44 Internal to libcapability, add the LC_IGNOREENTR flag, which is used by the RPC functions to prevent signal delivery from interrupting RPCs, leading to an early abort or killing of the sandbox due to incomplete data delivery on wakeup. This flag is not set for the non-RPC wrapper functions ({lch,lcs}_{recv,send}()) so that they behave as expected by applications using socket I/O. Remove BUGS note about signal handling issues for RPCs. Affected files ... .. //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability.c#10 edit .. //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_host_io.c#4 edit .. //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_internal.h#4 edit .. //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_io.3#4 edit .. //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_sandbox_io.c#4 edit Differences ... ==== //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability.c#10 (text+ko) ==== @@ -30,7 +30,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability.c#9 $ + * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability.c#10 $ */ #include @@ -133,29 +133,37 @@ } ssize_t -_lc_send(int fd, const void *msg, size_t len, int flags) +_lc_send(int fd, const void *msg, size_t len, int flags, int lc_flags) { + ssize_t retlen; if (fd == -1 || fd == 0) { errno = ECHILD; return (-1); } - return (send(fd, msg, len, flags)); + if (lc_flags & LC_IGNOREEINTR) { + do { + retlen = send(fd, msg, len, flags); + } while (retlen < 0 && errno == EINTR); + } else + retlen = send(fd, msg, len, flags); + return (retlen); } ssize_t -_lc_send_rights(int fd, const void *msg, size_t len, int flags, int *fdp, - int fdcount) +_lc_send_rights(int fd, const void *msg, size_t len, int flags, int lc_flags, + int *fdp, int fdcount) { char cmsgbuf[CMSG_SPACE(LIBCAPABILITY_SANDBOX_API_MAXRIGHTS * sizeof(int))]; struct cmsghdr *cmsg; struct msghdr msghdr; struct iovec iov; + ssize_t retlen; int i; if (fdcount == 0) - return (_lc_send(fd, msg, len, flags)); + return (_lc_send(fd, msg, len, flags, lc_flags)); if (fd == -1 || fd == 0) { errno = ECHILD; @@ -186,23 +194,36 @@ msghdr.msg_control = cmsg; msghdr.msg_controllen = cmsg->cmsg_len; - return (sendmsg(fd, &msghdr, flags)); + if (lc_flags & LC_IGNOREEINTR) { + do { + retlen = sendmsg(fd, &msghdr, flags); + } while (retlen < 0 && errno == EINTR); + } else + retlen = sendmsg(fd, &msghdr, flags); + return (retlen); } ssize_t -_lc_recv(int fd, void *buf, size_t len, int flags) +_lc_recv(int fd, void *buf, size_t len, int flags, int lc_flags) { + ssize_t retlen; if (fd == -1 || fd == 0) { errno = ESRCH; return (-1); } - return (recv(fd, buf, len, flags)); + if (lc_flags & LC_IGNOREEINTR) { + do { + retlen = recv(fd, buf, len, flags); + } while (retlen < 0 && errno == EINTR); + return (retlen); + } else + return (recv(fd, buf, len, flags)); } ssize_t -_lc_recv_rights(int fd, void *buf, size_t len, int flags, int *fdp, - int *fdcountp) +_lc_recv_rights(int fd, void *buf, size_t len, int flags, int lc_flags, + int *fdp, int *fdcountp) { char cmsgbuf[CMSG_SPACE(LIBCAPABILITY_SANDBOX_API_MAXRIGHTS * sizeof(int))]; @@ -211,7 +232,7 @@ ssize_t retlen; if (*fdcountp == 0) - return (_lc_recv(fd, buf, len, flags)); + return (_lc_recv(fd, buf, len, flags, lc_flags)); if (fd == -1 || fd == 0) { errno = ECHILD; @@ -234,7 +255,12 @@ msghdr.msg_control = cmsgbuf; msghdr.msg_controllen = sizeof(cmsgbuf); - retlen = recvmsg(fd, &msghdr, flags); + if (lc_flags & LC_IGNOREEINTR) { + do { + retlen = recvmsg(fd, &msghdr, flags); + } while (retlen < 0 && errno == EINTR); + } else + retlen = recvmsg(fd, &msghdr, flags); if (retlen < 0) return (-1); if (_lc_receive_rights(&msghdr, fdp, fdcountp) < 0) ==== //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_host_io.c#4 (text+ko) ==== @@ -30,7 +30,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_host_io.c#3 $ + * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_host_io.c#4 $ */ #include @@ -60,7 +60,7 @@ lch_send(struct lc_sandbox *lcsp, const void *msg, size_t len, int flags) { - return (_lc_send(lcsp->lcs_fd_sock, msg, len, flags)); + return (_lc_send(lcsp->lcs_fd_sock, msg, len, flags, 0)); } ssize_t @@ -68,7 +68,7 @@ int flags, int *fdp, int fdcount) { - return (_lc_send_rights(lcsp->lcs_fd_sock, msg, len, flags, fdp, + return (_lc_send_rights(lcsp->lcs_fd_sock, msg, len, flags, 0, fdp, fdcount)); } @@ -76,7 +76,7 @@ lch_recv(struct lc_sandbox *lcsp, void *buf, size_t len, int flags) { - return (_lc_recv(lcsp->lcs_fd_sock, buf, len, flags)); + return (_lc_recv(lcsp->lcs_fd_sock, buf, len, flags, 0)); } ssize_t @@ -84,7 +84,7 @@ int *fdp, int *fdcountp) { - return (_lc_recv_rights(lcsp->lcs_fd_sock, buf, len, flags, fdp, + return (_lc_recv_rights(lcsp->lcs_fd_sock, buf, len, flags, 0, fdp, fdcountp)); } @@ -121,10 +121,12 @@ * Send our header. */ if (req_fdp != NULL) - len = lch_send_rights(lcsp, &req_hdr, sizeof(req_hdr), 0, - req_fdp, req_fdcount); + len = _lc_send_rights(lcsp->lcs_fd_sock, &req_hdr, + sizeof(req_hdr), 0, LC_IGNOREEINTR, req_fdp, + req_fdcount); else - len = lch_send(lcsp, &req_hdr, sizeof(req_hdr), 0); + len = _lc_send(lcsp->lcs_fd_sock, &req_hdr, sizeof(req_hdr), + 0, LC_IGNOREEINTR); if (len < 0) return (-1); if (len != sizeof(req_hdr)) { @@ -136,7 +138,8 @@ * Send the user request. */ for (i = 0; i < reqcount; i++) { - len = lch_send(lcsp, req[i].iov_base, req[i].iov_len, 0); + len = _lc_send(lcsp->lcs_fd_sock, req[i].iov_base, + req[i].iov_len, 0, LC_IGNOREEINTR); if (len < 0) return (-1); if ((size_t)len != req[i].iov_len) { @@ -149,10 +152,12 @@ * Receive our header and validate. */ if (rep_fdp != NULL) - len = lch_recv_rights(lcsp, &rep_hdr, sizeof(rep_hdr), - MSG_WAITALL, rep_fdp, rep_fdcountp); + len = _lc_recv_rights(lcsp->lcs_fd_sock, &rep_hdr, + sizeof(rep_hdr), MSG_WAITALL, LC_IGNOREEINTR, rep_fdp, + rep_fdcountp); else - len = lch_recv(lcsp, &rep_hdr, sizeof(rep_hdr), MSG_WAITALL); + len = _lc_recv(lcsp->lcs_fd_sock, &rep_hdr, sizeof(rep_hdr), + MSG_WAITALL, LC_IGNOREEINTR); if (len < 0) return (-1); if (len != sizeof(rep_hdr)) { @@ -183,9 +188,9 @@ space = rep[i].iov_len - off; left = rep_hdr.lcrpc_rephdr_datalen - totlen; want = (space > left) ? space : left; - len = lch_recv(lcsp, + len = _lc_recv(lcsp->lcs_fd_sock, (u_char *)((uintptr_t)rep[i].iov_base + off), - want, MSG_WAITALL); + want, MSG_WAITALL, LC_IGNOREEINTR); if (len < 0) return (-1); if ((size_t)len != want) { ==== //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_internal.h#4 (text+ko) ==== @@ -30,7 +30,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_internal.h#3 $ + * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_internal.h#4 $ */ #ifndef _LIBCAPABILITY_INTERNAL_H_ @@ -46,15 +46,21 @@ pid_t lcs_pid; }; +/* + * Communications flags for recv/send calls (lc_flags). + */ +#define LC_IGNOREEINTR 0x00000001 + struct msghdr; void _lc_dispose_rights(int *fdp, int fdcount); int _lc_receive_rights(struct msghdr *msg, int *fdp, int *fdcountp); -ssize_t _lc_recv(int fd, void *buf, size_t len, int flags); -ssize_t _lc_recv_rights(int fd, void *buf, size_t len, int flags, int *fdp, - int *fdcountp); -ssize_t _lc_send(int fd, const void *msg, size_t len, int flags); +ssize_t _lc_recv(int fd, void *buf, size_t len, int flags, int lc_flags); +ssize_t _lc_recv_rights(int fd, void *buf, size_t len, int flags, + int lc_flags, int *fdp, int *fdcountp); +ssize_t _lc_send(int fd, const void *msg, size_t len, int flags, + int lc_flags); ssize_t _lc_send_rights(int fd, const void *msg, size_t len, int flags, - int *fdp, int fdcount); + int lc_flags, int *fdp, int fdcount); #endif /* !_LIBCAPABILITY_INTERNAL_H_ */ ==== //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_io.3#4 (text+ko) ==== @@ -197,9 +197,6 @@ WARNING: THIS IS EXPERIMENTAL SECURITY SOFTWARE THAT MUST NOT BE RELIED ON IN PRODUCTION SYSTEMS. IT WILL BREAK YOUR SOFTWARE IN NEW AND UNEXPECTED WAYS. .Pp -The socket calls used by the simple RPC system are not robust in the presence -of signal delivery, which should be fixed. -.Pp All sequence numbers will always have the value 0. This is fine from a retransmission perspective, as generally no retransmission should be required, but consumers should serialize use of the ==== //depot/projects/trustedbsd/capabilities/src/lib/libcapability/libcapability_sandbox_io.c#4 (text+ko) ==== @@ -52,7 +52,7 @@ lcs_recv(struct lc_host *lchp, void *buf, size_t len, int flags) { - return (_lc_recv(lchp->lch_fd_sock, buf, len, flags)); + return (_lc_recv(lchp->lch_fd_sock, buf, len, flags, 0)); } ssize_t @@ -60,7 +60,7 @@ int *fdp, int *fdcountp) { - return (_lc_recv_rights(lchp->lch_fd_sock, buf, len, flags, fdp, + return (_lc_recv_rights(lchp->lch_fd_sock, buf, len, flags, 0, fdp, fdcountp)); } @@ -68,7 +68,7 @@ lcs_send(struct lc_host *lchp, const void *msg, size_t len, int flags) { - return (_lc_send(lchp->lch_fd_sock, msg, len, flags)); + return (_lc_send(lchp->lch_fd_sock, msg, len, flags, 0)); } ssize_t @@ -76,7 +76,7 @@ int flags, int *fdp, int fdcount) { - return (_lc_send_rights(lchp->lch_fd_sock, msg, len, flags, fdp, + return (_lc_send_rights(lchp->lch_fd_sock, msg, len, flags, 0, fdp, fdcount)); } @@ -97,10 +97,12 @@ int error; if (fdp != NULL) - len = lcs_recv_rights(lchp, &req_hdr, sizeof(req_hdr), - MSG_WAITALL, fdp, fdcountp); + len = _lc_recv_rights(lchp->lch_fd_sock, &req_hdr, + sizeof(req_hdr), MSG_WAITALL, LC_IGNOREEINTR, fdp, + fdcountp); else - len = lcs_recv(lchp, &req_hdr, sizeof(req_hdr), MSG_WAITALL); + len = _lc_recv(lchp->lch_fd_sock, &req_hdr, sizeof(req_hdr), + MSG_WAITALL, LC_IGNOREEINTR); if (len < 0) return (-1); if (len == 0) { @@ -146,8 +148,9 @@ */ totlen = 0; while (totlen < req_hdr.lcrpc_reqhdr_datalen) { - len = lcs_recv(lchp, buffer + totlen, - req_hdr.lcrpc_reqhdr_datalen - totlen, MSG_WAITALL); + len = _lc_recv(lchp->lch_fd_sock, buffer + totlen, + req_hdr.lcrpc_reqhdr_datalen - totlen, MSG_WAITALL, + LC_IGNOREEINTR); if (len < 0) { error = errno; if (fdp != NULL) @@ -209,10 +212,11 @@ * Send our header. */ if (fdp != NULL) - len = lcs_send_rights(lchp, &rep_hdr, sizeof(rep_hdr), 0, - fdp, fdcount); + len = _lc_send_rights(lchp->lch_fd_sock, &rep_hdr, + sizeof(rep_hdr), 0, LC_IGNOREEINTR, fdp, fdcount); else - len = lcs_send(lchp, &rep_hdr, sizeof(rep_hdr), 0); + len = _lc_send(lchp->lch_fd_sock, &rep_hdr, sizeof(rep_hdr), + 0, LC_IGNOREEINTR); if (len < 0) return (-1); if (len != sizeof(rep_hdr)) { @@ -224,7 +228,8 @@ * Send user data. */ for (i = 0; i < repcount; i++) { - len = lcs_send(lchp, rep[i].iov_base, rep[i].iov_len, 0); + len = _lc_send(lchp->lch_fd_sock, rep[i].iov_base, + rep[i].iov_len, 0, LC_IGNOREEINTR); if (len < 0) return (-1); if ((size_t)len != rep[i].iov_len) {