From owner-svn-src-head@FreeBSD.ORG Tue Feb 17 23:54:09 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 081D832F; Tue, 17 Feb 2015 23:54:09 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E68BC9F1; Tue, 17 Feb 2015 23:54:08 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t1HNs8DR019463; Tue, 17 Feb 2015 23:54:08 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t1HNs6NT019455; Tue, 17 Feb 2015 23:54:06 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <201502172354.t1HNs6NT019455@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Tue, 17 Feb 2015 23:54:06 +0000 (UTC) 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 X-SVN-Group: head 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.18-1 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: Tue, 17 Feb 2015 23:54:09 -0000 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_ */