svn commit: r357471 - in head/sys: kern sys

[Mateusz Guzik]

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 *