Author: kib
Date: Tue Sep 3 19:49:40 2019
New Revision: 351784
URL: https://svnweb.freebsd.org/changeset/base/351784
Log:
MFC r350199:
Check and avoid overflow when incrementing fp->f_count in
fget_unlocked() and fhold().
Modified:
stable/12/sys/kern/kern_descrip.c
stable/12/sys/kern/sys_generic.c
stable/12/sys/kern/uipc_usrreq.c
stable/12/sys/sys/file.h
stable/12/sys/sys/refcount.h
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/sys/kern/kern_descrip.c
==============================================================================
--- stable/12/sys/kern/kern_descrip.c Tue Sep 3 19:48:23 2019
(r351783)
+++ stable/12/sys/kern/kern_descrip.c Tue Sep 3 19:49:40 2019
(r351784)
@@ -853,6 +853,10 @@ kern_dup(struct thread *td, u_int mode, int flags, int
goto unlock;
}
+ oldfde = &fdp->fd_ofiles[old];
+ if (!fhold(oldfde->fde_file))
+ goto unlock;
+
/*
* If the caller specified a file descriptor, make sure the file
* table is large enough to hold it, and grab it. Otherwise, just
@@ -861,13 +865,17 @@ kern_dup(struct thread *td, u_int mode, int flags, int
switch (mode) {
case FDDUP_NORMAL:
case FDDUP_FCNTL:
- if ((error = fdalloc(td, new, &new)) != 0)
+ if ((error = fdalloc(td, new, &new)) != 0) {
+ fdrop(oldfde->fde_file, td);
goto unlock;
+ }
break;
case FDDUP_MUSTREPLACE:
/* Target file descriptor must exist. */
- if (fget_locked(fdp, new) == NULL)
+ if (fget_locked(fdp, new) == NULL) {
+ fdrop(oldfde->fde_file, td);
goto unlock;
+ }
break;
case FDDUP_FIXED:
if (new >= fdp->fd_nfiles) {
@@ -886,6 +894,7 @@ kern_dup(struct thread *td, u_int mode, int flags, int
PROC_UNLOCK(p);
if (error != 0) {
error = EMFILE;
+ fdrop(oldfde->fde_file, td);
goto unlock;
}
}
@@ -901,8 +910,6 @@ kern_dup(struct thread *td, u_int mode, int flags, int
KASSERT(old != new, ("new fd is same as old"));
- oldfde = &fdp->fd_ofiles[old];
- fhold(oldfde->fde_file);
newfde = &fdp->fd_ofiles[new];
delfp = newfde->fde_file;
@@ -1915,12 +1922,14 @@ finstall(struct thread *td, struct file *fp, int *fd,
MPASS(fd != NULL);
+ if (!fhold(fp))
+ return (EBADF);
FILEDESC_XLOCK(fdp);
if ((error = fdalloc(td, 0, fd))) {
FILEDESC_XUNLOCK(fdp);
+ fdrop(fp, td);
return (error);
}
- fhold(fp);
_finstall(fdp, fp, *fd, flags, fcaps);
FILEDESC_XUNLOCK(fdp);
return (0);
@@ -2061,7 +2070,8 @@ fdcopy(struct filedesc *fdp)
for (i = 0; i <= fdp->fd_lastfile; ++i) {
ofde = &fdp->fd_ofiles[i];
if (ofde->fde_file == NULL ||
- (ofde->fde_file->f_ops->fo_flags & DFLAG_PASSABLE) == 0) {
+ (ofde->fde_file->f_ops->fo_flags & DFLAG_PASSABLE) == 0 ||
+ !fhold(ofde->fde_file)) {
if (newfdp->fd_freefile == -1)
newfdp->fd_freefile = i;
continue;
@@ -2069,7 +2079,6 @@ fdcopy(struct filedesc *fdp)
nfde = &newfdp->fd_ofiles[i];
*nfde = *ofde;
filecaps_copy(&ofde->fde_caps, &nfde->fde_caps, true);
- fhold(nfde->fde_file);
fdused_init(newfdp, i);
newfdp->fd_lastfile = i;
}
@@ -2122,10 +2131,13 @@ fdcopy_remapped(struct filedesc *fdp, const int *fds,
error = EINVAL;
goto bad;
}
+ if (!fhold(nfde->fde_file)) {
+ error = EBADF;
+ goto bad;
+ }
nfde = &newfdp->fd_ofiles[i];
*nfde = *ofde;
filecaps_copy(&ofde->fde_caps, &nfde->fde_caps, true);
- fhold(nfde->fde_file);
fdused_init(newfdp, i);
newfdp->fd_lastfile = i;
}
@@ -2167,9 +2179,9 @@ fdclearlocks(struct thread *td)
(p->p_leader->p_flag & P_ADVLOCK) != 0) {
for (i = 0; i <= fdp->fd_lastfile; i++) {
fp = fdp->fd_ofiles[i].fde_file;
- if (fp == NULL || fp->f_type != DTYPE_VNODE)
+ if (fp == NULL || fp->f_type != DTYPE_VNODE ||
+ !fhold(fp))
continue;
- fhold(fp);
FILEDESC_XUNLOCK(fdp);
lf.l_whence = SEEK_SET;
lf.l_start = 0;
@@ -2613,8 +2625,8 @@ fget_cap(struct thread *td, int fd, cap_rights_t *need
get_locked:
FILEDESC_SLOCK(fdp);
error = fget_cap_locked(fdp, fd, needrightsp, fpp, havecapsp);
- if (error == 0)
- fhold(*fpp);
+ if (error == 0 && !fhold(*fpp))
+ error = EBADF;
FILEDESC_SUNLOCK(fdp);
#endif
return (error);
@@ -2673,14 +2685,19 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights
* table before this fd was closed, so it possible that
* there is a stale fp pointer in cached version.
*/
- fdt = *(const struct fdescenttbl * const volatile
*)&(fdp->fd_files);
+ fdt = *(const struct fdescenttbl * const volatile *)
+ &(fdp->fd_files);
continue;
}
+ if (__predict_false(count + 1 < count))
+ return (EBADF);
+
/*
* Use an acquire barrier to force re-reading of fdt so it is
* refreshed for verification.
*/
- if (atomic_fcmpset_acq_int(&fp->f_count, &count, count + 1) ==
0)
+ if (__predict_false(atomic_fcmpset_acq_int(&fp->f_count,
+ &count, count + 1) == 0))
goto retry;
fdt = fdp->fd_files;
#ifdef CAPABILITIES
@@ -3065,7 +3082,11 @@ dupfdopen(struct thread *td, struct filedesc *fdp, int
FILEDESC_XUNLOCK(fdp);
return (EACCES);
}
- fhold(fp);
+ if (!fhold(fp)) {
+ fdunused(fdp, indx);
+ FILEDESC_XUNLOCK(fdp);
+ return (EBADF);
+ }
newfde = &fdp->fd_ofiles[indx];
oldfde = &fdp->fd_ofiles[dfd];
ioctls = filecaps_copy_prep(&oldfde->fde_caps);
Modified: stable/12/sys/kern/sys_generic.c
==============================================================================
--- stable/12/sys/kern/sys_generic.c Tue Sep 3 19:48:23 2019
(r351783)
+++ stable/12/sys/kern/sys_generic.c Tue Sep 3 19:49:40 2019
(r351784)
@@ -757,7 +757,11 @@ kern_ioctl(struct thread *td, int fd, u_long com, cadd
fp = NULL; /* fhold() was not called yet */
goto out;
}
- fhold(fp);
+ if (!fhold(fp)) {
+ error = EBADF;
+ fp = NULL;
+ goto out;
+ }
if (locked == LA_SLOCKED) {
FILEDESC_SUNLOCK(fdp);
locked = LA_UNLOCKED;
Modified: stable/12/sys/kern/uipc_usrreq.c
==============================================================================
--- stable/12/sys/kern/uipc_usrreq.c Tue Sep 3 19:48:23 2019
(r351783)
+++ stable/12/sys/kern/uipc_usrreq.c Tue Sep 3 19:49:40 2019
(r351784)
@@ -2154,7 +2154,7 @@ unp_internalize(struct mbuf **controlp, struct thread
struct timespec *ts;
void *data;
socklen_t clen, datalen;
- int i, error, *fdp, oldfds;
+ int i, j, error, *fdp, oldfds;
u_int newlen;
UNP_LINK_UNLOCK_ASSERT();
@@ -2237,6 +2237,19 @@ unp_internalize(struct mbuf **controlp, struct thread
goto out;
}
fdp = data;
+ for (i = 0; i < oldfds; i++, fdp++) {
+ if (!fhold(fdesc->fd_ofiles[*fdp].fde_file)) {
+ fdp = data;
+ for (j = 0; j < i; j++, fdp++) {
+ fdrop(fdesc->fd_ofiles[*fdp].
+ fde_file, td);
+ }
+ FILEDESC_SUNLOCK(fdesc);
+ error = EBADF;
+ goto out;
+ }
+ }
+ fdp = data;
fdep = (struct filedescent **)
CMSG_DATA(mtod(*controlp, struct cmsghdr *));
fdev = malloc(sizeof(*fdev) * oldfds, M_FILECAPS,
@@ -2440,7 +2453,6 @@ unp_internalize_fp(struct file *fp)
unp->unp_file = fp;
unp->unp_msgcount++;
}
- fhold(fp);
unp_rights++;
UNP_LINK_WUNLOCK();
}
@@ -2601,10 +2613,10 @@ unp_gc(__unused void *arg, int pending)
if ((unp->unp_gcflag & UNPGC_DEAD) != 0) {
f = unp->unp_file;
if (unp->unp_msgcount == 0 || f == NULL ||
- f->f_count != unp->unp_msgcount)
+ f->f_count != unp->unp_msgcount ||
+ !fhold(f))
continue;
unref[total++] = f;
- fhold(f);
KASSERT(total <= unp_unreachable,
("unp_gc: incorrect unreachable count."));
}
Modified: stable/12/sys/sys/file.h
==============================================================================
--- stable/12/sys/sys/file.h Tue Sep 3 19:48:23 2019 (r351783)
+++ stable/12/sys/sys/file.h Tue Sep 3 19:49:40 2019 (r351784)
@@ -284,8 +284,12 @@ _fnoop(void)
return (0);
}
-#define fhold(fp)
\
- (refcount_acquire(&(fp)->f_count))
+static __inline __result_use_check bool
+fhold(struct file *fp)
+{
+ return (refcount_acquire_checked(&fp->f_count));
+}
+
#define fdrop(fp, td)
\
(refcount_release(&(fp)->f_count) ? _fdrop((fp), (td)) : _fnoop())
Modified: stable/12/sys/sys/refcount.h
==============================================================================
--- stable/12/sys/sys/refcount.h Tue Sep 3 19:48:23 2019
(r351783)
+++ stable/12/sys/sys/refcount.h Tue Sep 3 19:49:40 2019
(r351784)
@@ -55,6 +55,20 @@ refcount_acquire(volatile u_int *count)
atomic_add_int(count, 1);
}
+static __inline __result_use_check bool
+refcount_acquire_checked(volatile u_int *count)
+{
+ u_int lcount;
+
+ for (lcount = *count;;) {
+ if (__predict_false(lcount + 1 < lcount))
+ return (false);
+ if (__predict_true(atomic_fcmpset_int(count, &lcount,
+ lcount + 1) == 1))
+ return (true);
+ }
+}
+
static __inline int
refcount_release(volatile u_int *count)
{
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
