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_ */
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to