d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

file: Avoid a read-after-free of fd tables in sysctl handlers

Browse Source 
Some loops access the fd table of a different process, and drop the
filedesc lock while iterating, so they check the table's refcount.
However, we access the table before the first iteration, in order to get
the number of table entries, and this access can be a use-after-free.

Fix the problem by checking the refcount before we start iterating.

Reported by:	pho
Reviewed by:	mjg
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D34575
This commit is contained in:
Mark Johnston 2022-03-17 12:54:37 -04:00
parent 7846554819
commit c702242292
1 changed files with 18 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
sys/kern/kern_descrip.c
View File

@ -4194,9 +4194,9 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS)
if (fdp == NULL)
continue;
FILEDESC_SLOCK(fdp);
if (refcount_load(&fdp->fd_refcnt) == 0)
goto nextproc;
FILEDESC_FOREACH_FP(fdp, n, fp) {
if (refcount_load(&fdp->fd_refcnt) == 0)
break;
xf.xf_fd = n;
xf.xf_file = (uintptr_t)fp;
xf.xf_data = (uintptr_t)fp->f_data;
@ -4207,9 +4207,16 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS)
xf.xf_offset = foffset_get(fp);
xf.xf_flag = fp->f_flag;
error = SYSCTL_OUT(req, &xf, sizeof(xf));
if (error)
/*
* There is no need to re-check the fdtable refcount
* here since the filedesc lock is not dropped in the
* loop body.
*/
if (error != 0)
break;
}
nextproc:
FILEDESC_SUNLOCK(fdp);
fddrop(fdp);
if (error)
@ -4469,9 +4476,9 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen,
if (pwd != NULL)
pwd_drop(pwd);
FILEDESC_SLOCK(fdp);
if (refcount_load(&fdp->fd_refcnt) == 0)
goto skip;
FILEDESC_FOREACH_FP(fdp, i, fp) {
if (refcount_load(&fdp->fd_refcnt) == 0)
break;
#ifdef CAPABILITIES
rights = *cap_rights(fdp, i);
#else /* !CAPABILITIES */
@ -4484,9 +4491,10 @@ kern_proc_filedesc_out(struct proc *p, struct sbuf *sb, ssize_t maxlen,
* loop continues.
*/
error = export_file_to_sb(fp, i, &rights, efbuf);
if (error != 0)
if (error != 0 || refcount_load(&fdp->fd_refcnt) == 0)
break;
}
skip:
FILEDESC_SUNLOCK(fdp);
fail:
if (fdp != NULL)
@ -4633,18 +4641,19 @@ sysctl_kern_proc_ofiledesc(SYSCTL_HANDLER_ARGS)
if (pwd != NULL)
pwd_drop(pwd);
FILEDESC_SLOCK(fdp);
if (refcount_load(&fdp->fd_refcnt) == 0)
goto skip;
FILEDESC_FOREACH_FP(fdp, i, fp) {
if (refcount_load(&fdp->fd_refcnt) == 0)
break;
export_file_to_kinfo(fp, i, NULL, kif, fdp,
KERN_FILEDESC_PACK_KINFO);
FILEDESC_SUNLOCK(fdp);
kinfo_to_okinfo(kif, okif);
error = SYSCTL_OUT(req, okif, sizeof(*okif));
FILEDESC_SLOCK(fdp);
if (error)
if (error != 0 || refcount_load(&fdp->fd_refcnt) == 0)
break;
}
skip:
FILEDESC_SUNLOCK(fdp);
fddrop(fdp);
pddrop(pdp);