Date: Mon, 3 Feb 2020 22:32:49 +0000 (UTC) From: Mateusz Guzik <mjg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357471 - in head/sys: kern sys Message-ID: <202002032232.013MWn4v087095@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 *
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202002032232.013MWn4v087095>