From nobody Thu Mar 17 19:40:12 2022 X-Original-To: dev-commits-src-all@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 B3E041A26A1D; Thu, 17 Mar 2022 19:40:12 +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 4KKHY84fYdz3GbC; Thu, 17 Mar 2022 19:40:12 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1647546012; 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=ufbv+oPWVxDg0/B6egfZOLAj01JJozc6rxCERMFDKFc=; b=Eha96aLdTCwgJrJI2poXg3iKjNbI1rKHGrznwWTvGLf3SWuXbEl1LVq76oXqVCF4GSkuq2 6XCALgjMIQWVDMFfet9irwGfDMvBwBSHwOlemrTRjsAfGBL/Q7NynT3u+uMrAGW0Ou5Xcj oc7GFCpmf9RHl4VejUkAScQZ55eETqjaSdRj6+XKSmK63jr2zHyNSUCbBUiAoN33xXKa9P kmWC6OjuXKewhz1JBQNzNZLV5NhJ8sWbjP+TQ2RQ+gG/hy5U9nsNOfdMDacyp1xu+cPsC6 2+Z3s4xmp3Gy6j0lc1mONzB+T0NxBgXCDJud/H0vck/cxnqZdJemJ9wU1gqFow== 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 7FCD810656; Thu, 17 Mar 2022 19:40:12 +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 22HJeCF6066322; Thu, 17 Mar 2022 19:40:12 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 22HJeCeV066316; Thu, 17 Mar 2022 19:40:12 GMT (envelope-from git) Date: Thu, 17 Mar 2022 19:40:12 GMT Message-Id: <202203171940.22HJeCeV066316@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: c70224229205 - main - file: Avoid a read-after-free of fd tables in sysctl handlers List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@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/main X-Git-Reftype: branch X-Git-Commit: c70224229205c756bf1c2007a6b96b37126eb047 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1647546012; 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=ufbv+oPWVxDg0/B6egfZOLAj01JJozc6rxCERMFDKFc=; b=dbNNFQQs6qQfxZ5hA8FKdX+Yh8f5bkhJTtxbj4ESW329+4N4Exex6oBNFyLp1lhE2AwZA3 X1l9hs3T8Xc/aMurcubI8Sa4mRuQ3azBdB1UyN9zv2e0CKdi5oWJWEKQb6gIPkxX06k177 r4zKLC02/vF9mkLzwnubYjaaZL1N3JN4QMEbscTeZoUNXbF4moyrrYthttOwDkwnjnLW2m Ha3Bbab1366ANqoUFEWT4sME+2z00xzheFaZlUTBd4f64xglxJJW4kg5dsyk8tn8TsftDW vRVIgWT8N3xEIm6S8l7cKPB8aacIPtasUxxquzD+PRrW47MarTZcAmgwD8B+yw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1647546012; a=rsa-sha256; cv=none; b=vhvgsOSh3CS9jGMgvqKNYsoUPjoDDY95qBLRVY+odBxBNdldJIl5WQ7VPJTkGYdzgf+doR thvzj/0WMOTQO0pmUpIoOxOh5dITh2Sh3NQvjOHcA63NsLrBX27t35a9Oz6u2x6Dw/PftX rQPX877R27rIiuzdOsgxJlzvzq4bRouxgwEBfva1rWhCTgE9UxoRFpdL6hlurobCuinxrh LIB23T35gu/V8G54h71hvC9iy0u5wMa7hxuMdd9JuJ0c5rGtMK1x8gCCd4A5j8RXCao0zZ ADJSE9XCDw94sozMEQNbKJd/QGXOqHfHPjy9mCPWLokoFonELLxteWzlXBMGJA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=c70224229205c756bf1c2007a6b96b37126eb047 commit c70224229205c756bf1c2007a6b96b37126eb047 Author: Mark Johnston AuthorDate: 2022-03-17 16:54:37 +0000 Commit: Mark Johnston CommitDate: 2022-03-17 19:39: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 MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D34575 --- sys/kern/kern_descrip.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index 07984b9a4640..da1eb24cf9c8 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -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);