From f17ef28674f13bc2199d4819123248f5997b435a Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Tue, 22 Feb 2022 16:54:17 +0100 Subject: [PATCH] fd: rename fget*_locked to fget*_noref This gets rid of the error prone naming where fget_unlocked returns with a ref held, while fget_locked requires a lock but provides nothing in terms of making sure the file lives past unlock. No functional changes. --- .../opensolaris/uts/common/dtrace/dtrace.c | 5 ++++- sys/compat/freebsd32/freebsd32_capability.c | 2 +- sys/kern/kern_descrip.c | 18 +++++++++--------- sys/kern/sys_capability.c | 14 +++++++------- sys/kern/sys_generic.c | 2 +- sys/kern/tty.c | 2 +- sys/kern/uipc_mqueue.c | 4 ++-- sys/kern/uipc_usrreq.c | 2 +- sys/sys/filedesc.h | 6 +++--- 9 files changed, 29 insertions(+), 26 deletions(-) diff --git a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c index 38612a088066..b2e7b01b125c 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c @@ -5664,7 +5664,10 @@ dtrace_dif_subr(uint_t subr, uint_t rd, uint64_t *regs, } fdp = curproc->p_fd; FILEDESC_SLOCK(fdp); - fp = fget_locked(fdp, fd); + /* + * XXXMJG this looks broken as no ref is taken. + */ + fp = fget_noref(fdp, fd); mstate->dtms_getf = fp; regs[rd] = (uintptr_t)fp; FILEDESC_SUNLOCK(fdp); diff --git a/sys/compat/freebsd32/freebsd32_capability.c b/sys/compat/freebsd32/freebsd32_capability.c index 2796d2e9384f..ebcd4a6629c7 100644 --- a/sys/compat/freebsd32/freebsd32_capability.c +++ b/sys/compat/freebsd32/freebsd32_capability.c @@ -104,7 +104,7 @@ freebsd32_cap_ioctls_get(struct thread *td, fdp = td->td_proc->p_fd; FILEDESC_SLOCK(fdp); - if (fget_locked(fdp, fd) == NULL) { + if (fget_noref(fdp, fd) == NULL) { error = EBADF; goto out; } diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index 259be785f88d..ed9d0bc3c253 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -518,7 +518,7 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg) case F_GETFD: error = EBADF; FILEDESC_SLOCK(fdp); - fde = fdeget_locked(fdp, fd); + fde = fdeget_noref(fdp, fd); if (fde != NULL) { td->td_retval[0] = (fde->fde_flags & UF_EXCLOSE) ? FD_CLOEXEC : 0; @@ -530,7 +530,7 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg) case F_SETFD: error = EBADF; FILEDESC_XLOCK(fdp); - fde = fdeget_locked(fdp, fd); + fde = fdeget_noref(fdp, fd); if (fde != NULL) { fde->fde_flags = (fde->fde_flags & ~UF_EXCLOSE) | (arg & FD_CLOEXEC ? UF_EXCLOSE : 0); @@ -874,7 +874,7 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg) } kif = malloc(sizeof(*kif), M_TEMP, M_WAITOK | M_ZERO); FILEDESC_SLOCK(fdp); - error = fget_cap_locked(fdp, fd, &cap_fcntl_rights, &fp, NULL); + error = fget_cap_noref(fdp, fd, &cap_fcntl_rights, &fp, NULL); if (error == 0 && fhold(fp)) { export_file_to_kinfo(fp, fd, NULL, kif, fdp, 0); FILEDESC_SUNLOCK(fdp); @@ -945,7 +945,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int old, int new) error = EBADF; FILEDESC_XLOCK(fdp); - if (fget_locked(fdp, old) == NULL) + if (fget_noref(fdp, old) == NULL) goto unlock; if (mode == FDDUP_FIXED && old == new) { td->td_retval[0] = new; @@ -1383,7 +1383,7 @@ kern_close(struct thread *td, int fd) fdp = td->td_proc->p_fd; FILEDESC_XLOCK(fdp); - if ((fp = fget_locked(fdp, fd)) == NULL) { + if ((fp = fget_noref(fdp, fd)) == NULL) { FILEDESC_XUNLOCK(fdp); return (EBADF); } @@ -2812,7 +2812,7 @@ finit_vnode(struct file *fp, u_int flag, void *data, struct fileops *ops) } int -fget_cap_locked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, +fget_cap_noref(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, struct file **fpp, struct filecaps *havecapsp) { struct filedescent *fde; @@ -2821,7 +2821,7 @@ fget_cap_locked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, FILEDESC_LOCK_ASSERT(fdp); *fpp = NULL; - fde = fdeget_locked(fdp, fd); + fde = fdeget_noref(fdp, fd); if (fde == NULL) { error = EBADF; goto out; @@ -2877,7 +2877,7 @@ fget_cap(struct thread *td, int fd, cap_rights_t *needrightsp, get_locked: FILEDESC_SLOCK(fdp); - error = fget_cap_locked(fdp, fd, needrightsp, fpp, havecapsp); + error = fget_cap_noref(fdp, fd, needrightsp, fpp, havecapsp); if (error == 0 && !fhold(*fpp)) error = EBADF; FILEDESC_SUNLOCK(fdp); @@ -3587,7 +3587,7 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode, * closed, then reject. */ FILEDESC_XLOCK(fdp); - if ((fp = fget_locked(fdp, dfd)) == NULL) { + if ((fp = fget_noref(fdp, dfd)) == NULL) { FILEDESC_XUNLOCK(fdp); return (EBADF); } diff --git a/sys/kern/sys_capability.c b/sys/kern/sys_capability.c index 6f97b74a0a53..2543a06e941d 100644 --- a/sys/kern/sys_capability.c +++ b/sys/kern/sys_capability.c @@ -239,7 +239,7 @@ kern_cap_rights_limit(struct thread *td, int fd, cap_rights_t *rights) fdp = td->td_proc->p_fd; FILEDESC_XLOCK(fdp); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); if (fdep == NULL) { FILEDESC_XUNLOCK(fdp); return (EBADF); @@ -325,7 +325,7 @@ sys___cap_rights_get(struct thread *td, struct __cap_rights_get_args *uap) fdp = td->td_proc->p_fd; FILEDESC_SLOCK(fdp); - if (fget_locked(fdp, fd) == NULL) { + if (fget_noref(fdp, fd) == NULL) { FILEDESC_SUNLOCK(fdp); return (EBADF); } @@ -367,7 +367,7 @@ cap_ioctl_check(struct filedesc *fdp, int fd, u_long cmd) KASSERT(fd >= 0 && fd < fdp->fd_nfiles, ("%s: invalid fd=%d", __func__, fd)); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); KASSERT(fdep != NULL, ("%s: invalid fd=%d", __func__, fd)); @@ -433,7 +433,7 @@ kern_cap_ioctls_limit(struct thread *td, int fd, u_long *cmds, size_t ncmds) fdp = td->td_proc->p_fd; FILEDESC_XLOCK(fdp); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); if (fdep == NULL) { error = EBADF; goto out; @@ -509,7 +509,7 @@ sys_cap_ioctls_get(struct thread *td, struct cap_ioctls_get_args *uap) } FILEDESC_SLOCK(fdp); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); if (fdep == NULL) { error = EBADF; FILEDESC_SUNLOCK(fdp); @@ -593,7 +593,7 @@ sys_cap_fcntls_limit(struct thread *td, struct cap_fcntls_limit_args *uap) fdp = td->td_proc->p_fd; FILEDESC_XLOCK(fdp); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); if (fdep == NULL) { FILEDESC_XUNLOCK(fdp); return (EBADF); @@ -626,7 +626,7 @@ sys_cap_fcntls_get(struct thread *td, struct cap_fcntls_get_args *uap) fdp = td->td_proc->p_fd; FILEDESC_SLOCK(fdp); - fdep = fdeget_locked(fdp, fd); + fdep = fdeget_noref(fdp, fd); if (fdep == NULL) { FILEDESC_SUNLOCK(fdp); return (EBADF); diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c index 276dd4acd26d..dd33d3dbe53e 100644 --- a/sys/kern/sys_generic.c +++ b/sys/kern/sys_generic.c @@ -748,7 +748,7 @@ kern_ioctl(struct thread *td, int fd, u_long com, caddr_t data) } #ifdef CAPABILITIES - if ((fp = fget_locked(fdp, fd)) == NULL) { + if ((fp = fget_noref(fdp, fd)) == NULL) { error = EBADF; goto out; } diff --git a/sys/kern/tty.c b/sys/kern/tty.c index 9e6cd8d317cc..5fa7b3dc7525 100644 --- a/sys/kern/tty.c +++ b/sys/kern/tty.c @@ -2081,7 +2081,7 @@ ttyhook_register(struct tty **rtp, struct proc *p, int fd, struct ttyhook *th, */ fdp = p->p_fd; FILEDESC_SLOCK(fdp); - error = fget_cap_locked(fdp, fd, cap_rights_init_one(&rights, CAP_TTYHOOK), + error = fget_cap_noref(fdp, fd, cap_rights_init_one(&rights, CAP_TTYHOOK), &fp, NULL); if (error == 0 && !fhold(fp)) error = EBADF; diff --git a/sys/kern/uipc_mqueue.c b/sys/kern/uipc_mqueue.c index f2e0240436f4..745c1174638e 100644 --- a/sys/kern/uipc_mqueue.c +++ b/sys/kern/uipc_mqueue.c @@ -2350,7 +2350,7 @@ kern_kmq_notify(struct thread *td, int mqd, struct sigevent *sigev) return (error); again: FILEDESC_SLOCK(fdp); - fp2 = fget_locked(fdp, mqd); + fp2 = fget_noref(fdp, mqd); if (fp2 == NULL) { FILEDESC_SUNLOCK(fdp); error = EBADF; @@ -2482,7 +2482,7 @@ mq_proc_exit(void *arg __unused, struct proc *p) fdp = p->p_fd; FILEDESC_SLOCK(fdp); for (i = 0; i < fdp->fd_nfiles; ++i) { - fp = fget_locked(fdp, i); + fp = fget_noref(fdp, i); if (fp != NULL && fp->f_ops == &mqueueops) { mq = FPTOMQ(fp); mtx_lock(&mq->mq_mutex); diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 360cd5424924..c2bf10b1f3e3 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -2262,7 +2262,7 @@ unp_internalize(struct mbuf **controlp, struct thread *td) fdp = data; FILEDESC_SLOCK(fdesc); for (i = 0; i < oldfds; i++, fdp++) { - fp = fget_locked(fdesc, *fdp); + fp = fget_noref(fdesc, *fdp); if (fp == NULL) { FILEDESC_SUNLOCK(fdesc); error = EBADF; diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h index 24eb6eb2c061..b64a4c4849d9 100644 --- a/sys/sys/filedesc.h +++ b/sys/sys/filedesc.h @@ -287,7 +287,7 @@ int getvnode_path(struct thread *td, int fd, cap_rights_t *rightsp, struct file **fpp); void mountcheckdirs(struct vnode *olddp, struct vnode *newdp); -int fget_cap_locked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, +int fget_cap_noref(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, struct file **fpp, struct filecaps *havecapsp); int fget_cap(struct thread *td, int fd, cap_rights_t *needrightsp, struct file **fpp, struct filecaps *havecapsp); @@ -304,7 +304,7 @@ int fget_only_user(struct filedesc *fdp, int fd, cap_rights_t *needrightsp, /* Requires a FILEDESC_{S,X}LOCK held and returns without a ref. */ static __inline struct file * -fget_locked(struct filedesc *fdp, int fd) +fget_noref(struct filedesc *fdp, int fd) { FILEDESC_LOCK_ASSERT(fdp); @@ -316,7 +316,7 @@ fget_locked(struct filedesc *fdp, int fd) } static __inline struct filedescent * -fdeget_locked(struct filedesc *fdp, int fd) +fdeget_noref(struct filedesc *fdp, int fd) { struct filedescent *fde;