From owner-svn-src-head@freebsd.org Mon Feb 3 22:32:50 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 493E0231051; Mon, 3 Feb 2020 22:32:50 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48BMz61CMbz4Dpb; Mon, 3 Feb 2020 22:32:50 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 24ADD7DE4; Mon, 3 Feb 2020 22:32:50 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 013MWoUK087097; Mon, 3 Feb 2020 22:32:50 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 013MWn4v087095; Mon, 3 Feb 2020 22:32:49 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202002032232.013MWn4v087095@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Mon, 3 Feb 2020 22:32:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357471 - in head/sys: kern sys X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: in head/sys: kern sys X-SVN-Commit-Revision: 357471 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Feb 2020 22:32:50 -0000 Author: mjg Date: Mon Feb 3 22:32:49 2020 New Revision: 357471 URL: https://svnweb.freebsd.org/changeset/base/357471 Log: fd: streamline fget_unlocked clang has the unfortunate property of paying little attention to prediction hints when faced with a loop spanning the majority of the rotuine. In particular fget_unlocked has an unlikely corner case where it starts almost from scratch. Faced with this clang generates a maze of taken jumps, whereas gcc produces jump-free code (in the expected case). Work around the problem by providing a variant which only tries once and resorts to calling the original code if anything goes wrong. While here note that the 'seq' parameter is almost never passed, thus the seldom users are redirected to call it directly. Modified: head/sys/kern/kern_descrip.c head/sys/sys/capsicum.h head/sys/sys/filedesc.h Modified: head/sys/kern/kern_descrip.c ============================================================================== --- head/sys/kern/kern_descrip.c Mon Feb 3 22:27:55 2020 (r357470) +++ head/sys/kern/kern_descrip.c Mon Feb 3 22:32:49 2020 (r357471) @@ -2766,6 +2766,69 @@ fget_unlocked_seq(struct filedesc *fdp, int fd, cap_ri } /* + * See the comments in fget_unlocked_seq for an explanation of how this works. + * + * This is a simplified variant which bails out to the aforementioned routine + * if anything goes wrong. In practice this will only happens when userspace + * is racing with itself. + */ +int +fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, + struct file **fpp) +{ +#ifdef CAPABILITIES + const struct filedescent *fde; +#endif + const struct fdescenttbl *fdt; + struct file *fp; +#ifdef CAPABILITIES + seqc_t seq; + const cap_rights_t *haverights; +#endif + + fdt = fdp->fd_files; + if (__predict_false((u_int)fd >= fdt->fdt_nfiles)) + return (EBADF); +#ifdef CAPABILITIES + seq = seqc_read_any(fd_seqc(fdt, fd)); + if (__predict_false(seqc_in_modify(seq))) + goto out_fallback; + fde = &fdt->fdt_ofiles[fd]; + haverights = cap_rights_fde_inline(fde); + fp = fde->fde_file; +#else + fp = fdt->fdt_ofiles[fd].fde_file; +#endif + if (__predict_false(fp == NULL)) + goto out_fallback; +#ifdef CAPABILITIES + if (__predict_false(cap_check_inline_transient(haverights, needrightsp))) + goto out_fallback; +#endif + if (__predict_false(!refcount_acquire_if_not_zero(&fp->f_count))) + goto out_fallback; + + /* + * Use an acquire barrier to force re-reading of fdt so it is + * refreshed for verification. + */ + atomic_thread_fence_acq(); + fdt = fdp->fd_files; +#ifdef CAPABILITIES + if (__predict_false(!seqc_consistent_nomb(fd_seqc(fdt, fd), seq))) +#else + if (__predict_false(fp != fdt->fdt_ofiles[fd].fde_file)) +#endif + goto out_fdrop; + *fpp = fp; + return (0); +out_fdrop: + fdrop(fp, curthread); +out_fallback: + return (fget_unlocked_seq(fdp, fd, needrightsp, fpp, NULL)); +} + +/* * Extract the file pointer associated with the specified descriptor for the * current user process. * Modified: head/sys/sys/capsicum.h ============================================================================== --- head/sys/sys/capsicum.h Mon Feb 3 22:27:55 2020 (r357470) +++ head/sys/sys/capsicum.h Mon Feb 3 22:32:49 2020 (r357471) @@ -351,8 +351,14 @@ void __cap_rights_sysinit(void *arg); _Static_assert(CAP_RIGHTS_VERSION == CAP_RIGHTS_VERSION_00, "unsupported version of capsicum rights"); +/* + * Allow checking caps which are possibly getting modified at the same time. + * The caller is expected to determine whether the result is legitimate via + * other means, see fget_unlocked for an example. + */ + static inline bool -cap_rights_contains(const cap_rights_t *big, const cap_rights_t *little) +cap_rights_contains_transient(const cap_rights_t *big, const cap_rights_t *little) { if (__predict_true( @@ -362,6 +368,8 @@ cap_rights_contains(const cap_rights_t *big, const cap return (false); } +#define cap_rights_contains cap_rights_contains_transient + int cap_check_failed_notcapable(const cap_rights_t *havep, const cap_rights_t *needp); @@ -371,6 +379,15 @@ cap_check_inline(const cap_rights_t *havep, const cap_ if (__predict_false(!cap_rights_contains(havep, needp))) return (cap_check_failed_notcapable(havep, needp)); + return (0); +} + +static inline int +cap_check_inline_transient(const cap_rights_t *havep, const cap_rights_t *needp) +{ + + if (__predict_false(!cap_rights_contains(havep, needp))) + return (1); return (0); } Modified: head/sys/sys/filedesc.h ============================================================================== --- head/sys/sys/filedesc.h Mon Feb 3 22:27:55 2020 (r357470) +++ head/sys/sys/filedesc.h Mon Feb 3 22:32:49 2020 (r357471) @@ -206,8 +206,8 @@ int fget_cap(struct thread *td, int fd, cap_rights_t * /* Return a referenced file from an unlocked descriptor. */ int fget_unlocked_seq(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, struct file **fpp, seqc_t *seqp); -#define fget_unlocked(fdp, fd, needrightsp, fpp) \ - fget_unlocked_seq(fdp, fd, needrightsp, fpp, NULL) +int fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, + struct file **fpp); /* Requires a FILEDESC_{S,X}LOCK held and returns without a ref. */ static __inline struct file *