Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Feb 2015 23:54:06 +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: r278930 - in head/sys: kern ofed/include/linux sys
Message-ID:  <201502172354.t1HNs6NT019455@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mjg
Date: Tue Feb 17 23:54:06 2015
New Revision: 278930
URL: https://svnweb.freebsd.org/changeset/base/278930

Log:
  filedesc: simplify fget_unlocked & friends
  
  Introduce fget_fcntl which performs appropriate checks when needed.
  This removes a branch from fget_unlocked.
  
  Introduce fget_mmap dealing with cap_rights_to_vmprot conversion.
  This removes a branch from _fget.
  
  Modify fget_unlocked to pass sequence counter to interested callers so
  that they can perform their own checks and make sure the result was
  otained from stable & current state.
  
  Reviewed by:	silence on -hackers

Modified:
  head/sys/kern/kern_descrip.c
  head/sys/kern/sys_generic.c
  head/sys/kern/tty.c
  head/sys/kern/uipc_syscalls.c
  head/sys/kern/vfs_syscalls.c
  head/sys/ofed/include/linux/file.h
  head/sys/sys/file.h
  head/sys/sys/filedesc.h

Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/kern/kern_descrip.c	Tue Feb 17 23:54:06 2015	(r278930)
@@ -531,8 +531,8 @@ kern_fcntl(struct thread *td, int fd, in
 		break;
 
 	case F_GETFL:
-		error = fget_unlocked(fdp, fd,
-		    cap_rights_init(&rights, CAP_FCNTL), F_GETFL, &fp, NULL);
+		error = fget_fcntl(td, fd,
+		    cap_rights_init(&rights, CAP_FCNTL), F_GETFL, &fp);
 		if (error != 0)
 			break;
 		td->td_retval[0] = OFLAGS(fp->f_flag);
@@ -540,8 +540,8 @@ kern_fcntl(struct thread *td, int fd, in
 		break;
 
 	case F_SETFL:
-		error = fget_unlocked(fdp, fd,
-		    cap_rights_init(&rights, CAP_FCNTL), F_SETFL, &fp, NULL);
+		error = fget_fcntl(td, fd,
+		    cap_rights_init(&rights, CAP_FCNTL), F_SETFL, &fp);
 		if (error != 0)
 			break;
 		do {
@@ -568,8 +568,8 @@ kern_fcntl(struct thread *td, int fd, in
 		break;
 
 	case F_GETOWN:
-		error = fget_unlocked(fdp, fd,
-		    cap_rights_init(&rights, CAP_FCNTL), F_GETOWN, &fp, NULL);
+		error = fget_fcntl(td, fd,
+		    cap_rights_init(&rights, CAP_FCNTL), F_GETOWN, &fp);
 		if (error != 0)
 			break;
 		error = fo_ioctl(fp, FIOGETOWN, &tmp, td->td_ucred, td);
@@ -579,8 +579,8 @@ kern_fcntl(struct thread *td, int fd, in
 		break;
 
 	case F_SETOWN:
-		error = fget_unlocked(fdp, fd,
-		    cap_rights_init(&rights, CAP_FCNTL), F_SETOWN, &fp, NULL);
+		error = fget_fcntl(td, fd,
+		    cap_rights_init(&rights, CAP_FCNTL), F_SETOWN, &fp);
 		if (error != 0)
 			break;
 		tmp = arg;
@@ -602,7 +602,7 @@ kern_fcntl(struct thread *td, int fd, in
 	case F_SETLK:
 	do_setlk:
 		cap_rights_init(&rights, CAP_FLOCK);
-		error = fget_unlocked(fdp, fd, &rights, 0, &fp, NULL);
+		error = fget_unlocked(fdp, fd, &rights, &fp, NULL);
 		if (error != 0)
 			break;
 		if (fp->f_type != DTYPE_VNODE) {
@@ -691,7 +691,7 @@ kern_fcntl(struct thread *td, int fd, in
 		 * that the closing thread was a bit slower and that the
 		 * advisory lock succeeded before the close.
 		 */
-		error = fget_unlocked(fdp, fd, &rights, 0, &fp2, NULL);
+		error = fget_unlocked(fdp, fd, &rights, &fp2, NULL);
 		if (error != 0) {
 			fdrop(fp, td);
 			break;
@@ -710,7 +710,7 @@ kern_fcntl(struct thread *td, int fd, in
 
 	case F_GETLK:
 		error = fget_unlocked(fdp, fd,
-		    cap_rights_init(&rights, CAP_FLOCK), 0, &fp, NULL);
+		    cap_rights_init(&rights, CAP_FLOCK), &fp, NULL);
 		if (error != 0)
 			break;
 		if (fp->f_type != DTYPE_VNODE) {
@@ -748,7 +748,7 @@ kern_fcntl(struct thread *td, int fd, in
 		arg = arg ? 128 * 1024: 0;
 		/* FALLTHROUGH */
 	case F_READAHEAD:
-		error = fget_unlocked(fdp, fd, NULL, 0, &fp, NULL);
+		error = fget_unlocked(fdp, fd, NULL, &fp, NULL);
 		if (error != 0)
 			break;
 		if (fp->f_type != DTYPE_VNODE) {
@@ -2327,10 +2327,10 @@ finit(struct file *fp, u_int flag, short
 
 int
 fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
-    int needfcntl, struct file **fpp, cap_rights_t *haverightsp)
+    struct file **fpp, seq_t *seqp)
 {
 #ifdef CAPABILITIES
-	struct filedescent fde;
+	struct filedescent *fde;
 #endif
 	struct fdescenttbl *fdt;
 	struct file *fp;
@@ -2355,28 +2355,23 @@ fget_unlocked(struct filedesc *fdp, int 
 	for (;;) {
 #ifdef CAPABILITIES
 		seq = seq_read(fd_seq(fdt, fd));
-		fde = fdt->fdt_ofiles[fd];
+		fde = &fdt->fdt_ofiles[fd];
+		haverights = cap_rights_fde(fde);
+		fp = fde->fde_file;
 		if (!seq_consistent(fd_seq(fdt, fd), seq)) {
 			cpu_spinwait();
 			continue;
 		}
-		fp = fde.fde_file;
 #else
 		fp = fdt->fdt_ofiles[fd].fde_file;
 #endif
 		if (fp == NULL)
 			return (EBADF);
 #ifdef CAPABILITIES
-		haverights = cap_rights_fde(&fde);
 		if (needrightsp != NULL) {
 			error = cap_check(haverights, needrightsp);
 			if (error != 0)
 				return (error);
-			if (cap_rights_is_set(needrightsp, CAP_FCNTL)) {
-				error = cap_fcntl_check_fde(&fde, needfcntl);
-				if (error != 0)
-					return (error);
-			}
 		}
 #endif
 	retry:
@@ -2406,11 +2401,9 @@ fget_unlocked(struct filedesc *fdp, int 
 		fdrop(fp, curthread);
 	}
 	*fpp = fp;
-	if (haverightsp != NULL) {
+	if (seqp != NULL) {
 #ifdef CAPABILITIES
-		*haverightsp = *haverights;
-#else
-		CAP_ALL(haverightsp);
+		*seqp = seq;
 #endif
 	}
 	return (0);
@@ -2431,11 +2424,11 @@ fget_unlocked(struct filedesc *fdp, int 
  */
 static __inline int
 _fget(struct thread *td, int fd, struct file **fpp, int flags,
-    cap_rights_t *needrightsp, u_char *maxprotp)
+    cap_rights_t *needrightsp, seq_t *seqp)
 {
 	struct filedesc *fdp;
 	struct file *fp;
-	cap_rights_t haverights, needrights;
+	cap_rights_t needrights;
 	int error;
 
 	*fpp = NULL;
@@ -2444,9 +2437,7 @@ _fget(struct thread *td, int fd, struct 
 		needrights = *needrightsp;
 	else
 		cap_rights_init(&needrights);
-	if (maxprotp != NULL)
-		cap_rights_set(&needrights, CAP_MMAP);
-	error = fget_unlocked(fdp, fd, &needrights, 0, &fp, &haverights);
+	error = fget_unlocked(fdp, fd, &needrights, &fp, seqp);
 	if (error != 0)
 		return (error);
 	if (fp->f_ops == &badfileops) {
@@ -2454,17 +2445,6 @@ _fget(struct thread *td, int fd, struct 
 		return (EBADF);
 	}
 
-#ifdef CAPABILITIES
-	/*
-	 * If requested, convert capability rights to access flags.
-	 */
-	if (maxprotp != NULL)
-		*maxprotp = cap_rights_to_vmprot(&haverights);
-#else /* !CAPABILITIES */
-	if (maxprotp != NULL)
-		*maxprotp = VM_PROT_ALL;
-#endif /* CAPABILITIES */
-
 	/*
 	 * FREAD and FWRITE failure return EBADF as per POSIX.
 	 */
@@ -2506,8 +2486,31 @@ int
 fget_mmap(struct thread *td, int fd, cap_rights_t *rightsp, u_char *maxprotp,
     struct file **fpp)
 {
+	int error;
+#ifndef CAPABILITIES
+	error = _fget(td, fd, fpp, 0, rightsp, NULL);
+	if (maxprotp != NULL)
+		*maxprotp = VM_PROT_ALL;
+#else
+	struct filedesc *fdp = td->td_proc->p_fd;
+	seq_t seq;
 
-	return (_fget(td, fd, fpp, 0, rightsp, maxprotp));
+	MPASS(cap_rights_is_set(rightsp, CAP_MMAP));
+	for (;;) {
+		error = _fget(td, fd, fpp, 0, rightsp, &seq);
+		if (error != 0)
+			return (error);
+		/*
+		 * If requested, convert capability rights to access flags.
+		 */
+		if (maxprotp != NULL)
+			*maxprotp = cap_rights_to_vmprot(cap_rights(fdp, fd));
+		if (!fd_modified(td->td_proc->p_fd, fd, seq))
+			break;
+		fdrop(*fpp, td);
+	}
+#endif
+	return (error);
 }
 
 int
@@ -2524,6 +2527,35 @@ fget_write(struct thread *td, int fd, ca
 	return (_fget(td, fd, fpp, FWRITE, rightsp, NULL));
 }
 
+int
+fget_fcntl(struct thread *td, int fd, cap_rights_t *rightsp, int needfcntl,
+    struct file **fpp)
+{
+	struct filedesc *fdp = td->td_proc->p_fd;
+#ifndef CAPABILITIES
+	return (fget_unlocked(fdp, fd, rightsp, fpp, NULL));
+#else
+	int error;
+	seq_t seq;
+
+	MPASS(cap_rights_is_set(rightsp, CAP_FCNTL));
+	for (;;) {
+		error = fget_unlocked(fdp, fd, rightsp, fpp, &seq);
+		if (error != 0)
+			return (error);
+		error = cap_fcntl_check(fdp, fd, needfcntl);
+		if (!fd_modified(fdp, fd, seq))
+			break;
+		fdrop(*fpp, td);
+	}
+	if (error != 0) {
+		fdrop(*fpp, td);
+		*fpp = NULL;
+	}
+	return (error);
+#endif
+}
+
 /*
  * Like fget() but loads the underlying vnode, or returns an error if the
  * descriptor does not represent a vnode.  Note that pipes use vnodes but

Modified: head/sys/kern/sys_generic.c
==============================================================================
--- head/sys/kern/sys_generic.c	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/kern/sys_generic.c	Tue Feb 17 23:54:06 2015	(r278930)
@@ -1214,7 +1214,7 @@ getselfd_cap(struct filedesc *fdp, int f
 
 	cap_rights_init(&rights, CAP_EVENT);
 
-	return (fget_unlocked(fdp, fd, &rights, 0, fpp, NULL));
+	return (fget_unlocked(fdp, fd, &rights, fpp, NULL));
 }
 
 /*

Modified: head/sys/kern/tty.c
==============================================================================
--- head/sys/kern/tty.c	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/kern/tty.c	Tue Feb 17 23:54:06 2015	(r278930)
@@ -1897,7 +1897,7 @@ ttyhook_register(struct tty **rtp, struc
 	/* Validate the file descriptor. */
 	fdp = p->p_fd;
 	error = fget_unlocked(fdp, fd, cap_rights_init(&rights, CAP_TTYHOOK),
-	    0, &fp, NULL);
+	    &fp, NULL);
 	if (error != 0)
 		return (error);
 	if (fp->f_ops == &badfileops) {

Modified: head/sys/kern/uipc_syscalls.c
==============================================================================
--- head/sys/kern/uipc_syscalls.c	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/kern/uipc_syscalls.c	Tue Feb 17 23:54:06 2015	(r278930)
@@ -156,7 +156,7 @@ getsock_cap(struct filedesc *fdp, int fd
 	struct file *fp;
 	int error;
 
-	error = fget_unlocked(fdp, fd, rightsp, 0, &fp, NULL);
+	error = fget_unlocked(fdp, fd, rightsp, &fp, NULL);
 	if (error != 0)
 		return (error);
 	if (fp->f_type != DTYPE_SOCKET) {

Modified: head/sys/kern/vfs_syscalls.c
==============================================================================
--- head/sys/kern/vfs_syscalls.c	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/kern/vfs_syscalls.c	Tue Feb 17 23:54:06 2015	(r278930)
@@ -4217,7 +4217,7 @@ getvnode(struct filedesc *fdp, int fd, c
 	struct file *fp;
 	int error;
 
-	error = fget_unlocked(fdp, fd, rightsp, 0, &fp, NULL);
+	error = fget_unlocked(fdp, fd, rightsp, &fp, NULL);
 	if (error != 0)
 		return (error);
 

Modified: head/sys/ofed/include/linux/file.h
==============================================================================
--- head/sys/ofed/include/linux/file.h	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/ofed/include/linux/file.h	Tue Feb 17 23:54:06 2015	(r278930)
@@ -48,7 +48,7 @@ linux_fget(unsigned int fd)
 {
 	struct file *file;
 
-	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, 0, &file,
+	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, &file,
 	    NULL) != 0) {
 		return (NULL);
 	}
@@ -73,7 +73,7 @@ put_unused_fd(unsigned int fd)
 {
 	struct file *file;
 
-	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, 0, &file,
+	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, &file,
 	    NULL) != 0) {
 		return;
 	}
@@ -93,7 +93,7 @@ fd_install(unsigned int fd, struct linux
 {
 	struct file *file;
 
-	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, 0, &file,
+	if (fget_unlocked(curthread->td_proc->p_fd, fd, NULL, &file,
 	    NULL) != 0) {
 		file = NULL;
 	}

Modified: head/sys/sys/file.h
==============================================================================
--- head/sys/sys/file.h	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/sys/file.h	Tue Feb 17 23:54:06 2015	(r278930)
@@ -230,6 +230,8 @@ int fget_read(struct thread *td, int fd,
     struct file **fpp);
 int fget_write(struct thread *td, int fd, cap_rights_t *rightsp,
     struct file **fpp);
+int fget_fcntl(struct thread *td, int fd, cap_rights_t *rightsp,
+    int needfcntl, struct file **fpp);
 int _fdrop(struct file *fp, struct thread *td);
 
 fo_rdwr_t	invfo_rdwr;

Modified: head/sys/sys/filedesc.h
==============================================================================
--- head/sys/sys/filedesc.h	Tue Feb 17 23:41:08 2015	(r278929)
+++ head/sys/sys/filedesc.h	Tue Feb 17 23:54:06 2015	(r278930)
@@ -169,7 +169,7 @@ void	mountcheckdirs(struct vnode *olddp,
 
 /* Return a referenced file from an unlocked descriptor. */
 int	fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
-	    int needfcntl, struct file **fpp, cap_rights_t *haverightsp);
+	    struct file **fpp, seq_t *seqp);
 
 /* Requires a FILEDESC_{S,X}LOCK held and returns without a ref. */
 static __inline struct file *
@@ -184,6 +184,13 @@ fget_locked(struct filedesc *fdp, int fd
 	return (fdp->fd_ofiles[fd].fde_file);
 }
 
+static __inline bool
+fd_modified(struct filedesc *fdp, int fd, seq_t seq)
+{
+
+	return (!seq_consistent(fd_seq(fdp->fd_files, fd), seq));
+}
+
 #endif /* _KERNEL */
 
 #endif /* !_SYS_FILEDESC_H_ */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201502172354.t1HNs6NT019455>