From nobody Thu Mar 31 15:32:55 2022 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 7B2F21A55C45; Thu, 31 Mar 2022 15:32:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4KTnPM2gcNz4Sh4; Thu, 31 Mar 2022 15:32:55 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648740775; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=P5aOKQNTyB/9koLB2m/UEFjfjGIaZoXG2d45lmeTaDM=; b=CuCxkM+EoThpMxjBYXxjoQ8dxmn3wJKtl5btOW4lNJEsYKhhbhhxzVNE3+JkVev2Na2ziV 6EMjEin2U9o9PHtyP6X+v7Rk7wXYK/IL4xSIhuOByPYFr8U3izqTH8d0dyfZsGYthDMu1k YPbKPdb0alkpJAnjBM66CTQdqTwuO6m9RTyfcUKWZHCSM0zacq2vQpVf0yVKmMkZNbY/Ku nNSzcP1jDjtjMuJStk8oAJWucEPrtsRtNo34uPSzgG/9/T1HrEIAJbVsMI6LRMLdFajSEQ vviNcUncvdgwCdmbaB2CRgcliaxuI/kq+O6aMQZZ35vgU97flNh0GhyGEjtRRQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 31F6B2A426; Thu, 31 Mar 2022 15:32:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 22VFWtam053378; Thu, 31 Mar 2022 15:32:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 22VFWtqx053377; Thu, 31 Mar 2022 15:32:55 GMT (envelope-from git) Date: Thu, 31 Mar 2022 15:32:55 GMT Message-Id: <202203311532.22VFWtqx053377@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: eef123d5fd97 - stable/13 - file: Avoid a read-after-free of fd tables in sysctl handlers List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: eef123d5fd97ef6d5c236f6420c1901b7e19499f Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648740775; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=P5aOKQNTyB/9koLB2m/UEFjfjGIaZoXG2d45lmeTaDM=; b=fumFGO84Ju8EnO+CoTN75+YjNusdQgaay+TpoNc6v2NZcz37de/04Mm7rSs0mYY7cn1WW+ 5K1ESWs7H9PBwzpZb86aBD3m6QUPNbx0VG3vWXbJF19rGk3vXwumk4XiBt2P2opTHiaQNO tEjQp8fS2y32A9TyuykT3OKAHs50TV7hIDrK5x+P+YlOwbBURrorfyWndJe8jONLc3Syt8 LFyWZMsUVCko976cWy3TJnOkAEwQ8CYM+1WLWrkLKYu6k/rwf64NIP6KN/z5cBdFbKwlaa MVvjN/1e/p38Em2ttWyivlcEW5mCLWv6Am6EQlmNNIck3gWCLi6WDuOi4Gs7tw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1648740775; a=rsa-sha256; cv=none; b=fQ0EHEwipewZGKJRGskHd+4fS8/LmuokjOF8jn6XydFmDNHikobcvFjn6UMND0SjqSEjNJ l2cC+UoOJzaVsMn8KOB7DKDJaDzI8XdcfAjcC9MJ6lB/3AIjfF9HiHNcXxEvc4fF74r9x0 wVTBqNHZ9o+QdVGJszEHhh4w+BbvusBScjdPsBik4Msysn212W3HhfESCUqOMim6BEIzZl hDvXJoP0GYylBiohpgJNTVRBk4qGaahcUGNrXqfATlzVUFR/HfGe/834tq+uCXfl0gQTjJ VgJfVL/yx1zwXpX6rlK1o2OG9hwOwzZrA9e76ugY8MI6cxv/B7vGJ59RThBZxw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=eef123d5fd97ef6d5c236f6420c1901b7e19499f commit eef123d5fd97ef6d5c236f6420c1901b7e19499f Author: Mark Johnston AuthorDate: 2022-03-17 16:54:37 +0000 Commit: Mark Johnston CommitDate: 2022-03-31 15:32:00 +0000 file: Avoid a read-after-free of fd tables in sysctl handlers 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 Sponsored by: The FreeBSD Foundation (cherry picked from commit c70224229205c756bf1c2007a6b96b37126eb047) --- sys/kern/kern_descrip.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index 03498d5a9ce9..de70be1a997f 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -4272,6 +4272,8 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS) if (fdp == NULL) continue; FILEDESC_SLOCK(fdp); + if (refcount_load(&fdp->fd_refcnt) == 0) + goto nextproc; lastfile = fdlastfile(fdp); for (n = 0; refcount_load(&fdp->fd_refcnt) > 0 && n <= lastfile; n++) { @@ -4287,9 +4289,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) @@ -4549,8 +4558,10 @@ 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; lastfile = fdlastfile(fdp); - for (i = 0; refcount_load(&fdp->fd_refcnt) > 0 && i <= lastfile; i++) { + for (i = 0; i <= lastfile; i++) { if ((fp = fdp->fd_ofiles[i].fde_file) == NULL) continue; #ifdef CAPABILITIES @@ -4565,9 +4576,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) @@ -4714,8 +4726,10 @@ 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; lastfile = fdlastfile(fdp); - for (i = 0; refcount_load(&fdp->fd_refcnt) > 0 && i <= lastfile; i++) { + for (i = 0; i <= lastfile; i++) { if ((fp = fdp->fd_ofiles[i].fde_file) == NULL) continue; export_file_to_kinfo(fp, i, NULL, kif, fdp, @@ -4724,9 +4738,10 @@ sysctl_kern_proc_ofiledesc(SYSCTL_HANDLER_ARGS) 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);