From nobody Fri Aug 26 07:10:27 2022 X-Original-To: dev-commits-src-main@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 4MDWFH5qqCz4ZR8Y; Fri, 26 Aug 2022 07:10:27 +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 4MDWFH5Jcbz3d3h; Fri, 26 Aug 2022 07:10:27 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661497827; 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=A4sIfQlWir/N1Y3WQWZ8EV47M6aHwVOt74lqhsLDC2Q=; b=BKyuS4AQvhP7K482jWf4iEvOS2bAiiGOvGVEgwGmzFeZIevtlD50AJuOBFkTdcsXMRA8ut zcVVjsJbwqjGKNVDpbM1ltWWRgPHWq4GQ6vxbJe+r0POWd8v1UKW5ThIRTvskATdGAvJKr afyQrqlJY0RvK0XYWAs3hnjkHxHeJOjzxYH7BHbHb/szGYtTUYo8N1Opa7epb2ExU5V9Cy /GCzAXbqF6VceOCbUljU08vTGGx3WWkNblI3KIHYVueT/mHGB77REBbOqcm1+CWe41EscN Q0cuRFUY7Gq4tm7FVBGUw3cdX7MdIg9IlBoFaHuLog3NrrbWQyXTg8TgJJd5bg== 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 4MDWFH3xzxzJGB; Fri, 26 Aug 2022 07:10:27 +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 27Q7ARq6097392; Fri, 26 Aug 2022 07:10:27 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 27Q7AR6t097391; Fri, 26 Aug 2022 07:10:27 GMT (envelope-from git) Date: Fri, 26 Aug 2022 07:10:27 GMT Message-Id: <202208260710.27Q7AR6t097391@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kirk McKusick Subject: git: f0be378a66a7 - main - Updates to UFS/FFS superblock integrity checks when reading a superblock. List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mckusick X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: f0be378a66a75ebf335e9388ef0d319a70064d94 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661497827; 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=A4sIfQlWir/N1Y3WQWZ8EV47M6aHwVOt74lqhsLDC2Q=; b=V1by2PfKRxdYhcmZIC06+D0tfG00B2ptVaXJnlJjCNEIlUaMGfC/ewwnjq2oUC8sgs+3Iy bH+G28LtRMIcr5IiG/JKoRIJ+xAkG+fNUzr8ThvBdELPR1Ks2tU4JH3LOVBZ6h90hM8y3t 7Izwzihmgxc6yAtoZA8joHvtQR2rYaTSZYjQvWPrC1mfbecVyIVJF69s0vo136yuKCsLs3 X94vzkFMXq5PfzmzdYWHDAVbMlKQdmRb6G85nquOkaKtrpWey4jZwNCrSGlGeZRuWd2pz3 BXF7QSIElHYLmlIjJTq4NFJPWxgELm3CXqilHMx1SMXV0xWFbZVawgOfdNNZtg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1661497827; a=rsa-sha256; cv=none; b=INvmM4pJt4Qors8NSZTg46de3Z3/RUxpOB/lKpQs7cBadjKrjLS3mgWjb9KA0yvUzJjB4r ESmnjzcI6to0HN9l+3tNSUfK9D/uLli7mRxH/huenPYCkM2RWPB25O1VsTbFrjV2ePwKd0 /9XGZEwq+ELbeWnwpJ0mX2hG70LDTxB1hgZLeg8TMc8Lci0g5NgFJbD62XW9/FZP08mdYm yUaz4qEJnKtkjxwK5xnMiP3zKTIjQ6Bnk4n/hfg+bJ1DAH+hAvVzy+N37/Lmxl/QD2+ZTU GFf0pmpHYdaVJfm76sSSwAHaZPio05ZQlQtJTDj1mX6r0aFjKa6F3Ts3KRsR5A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mckusick: URL: https://cgit.FreeBSD.org/src/commit/?id=f0be378a66a75ebf335e9388ef0d319a70064d94 commit f0be378a66a75ebf335e9388ef0d319a70064d94 Author: Kirk McKusick AuthorDate: 2022-08-26 07:09:01 +0000 Commit: Kirk McKusick CommitDate: 2022-08-26 07:09:01 +0000 Updates to UFS/FFS superblock integrity checks when reading a superblock. Further updates based on ways Peter Holm found to corrupt UFS superblocks in ways that could cause kernel hangs or crashes. No legitimate superblocks should fail as a result of these changes. Reported by: Peter Holm Tested by: Peter Holm Sponsored by: The FreeBSD Foundation --- sys/ufs/ffs/ffs_subr.c | 70 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c index 8c081313f873..f6c27cb38d62 100644 --- a/sys/ufs/ffs/ffs_subr.c +++ b/sys/ufs/ffs/ffs_subr.c @@ -333,6 +333,8 @@ readsuper(void *devfd, struct fs **fsp, off_t sblockloc, int flags, MPRINT("UFS%d superblock failed: %s (" #fmt ") %s %s (" \ #fmt ")\n", fs->fs_magic == FS_UFS1_MAGIC ? 1 : 2, \ #lhs, (intmax_t)lhs, #op, #rhs, (intmax_t)rhs); \ + if (error < 0) \ + return (ENOENT); \ if (error == 0) \ error = ENOENT; \ } @@ -351,6 +353,8 @@ readsuper(void *devfd, struct fs **fsp, off_t sblockloc, int flags, fs->fs_magic == FS_UFS1_MAGIC ? 1 : 2, #lhs1, \ (intmax_t)lhs1, #op1, #rhs1, (intmax_t)rhs1, #lhs2, \ (intmax_t)lhs2, #op2, #rhs2, (intmax_t)rhs2); \ + if (error < 0) \ + return (ENOENT); \ if (error == 0) \ error = ENOENT; \ } @@ -385,12 +389,16 @@ validate_sblock(struct fs *fs, int flags) */ if ((flags & UFS_FSRONLY) == UFS_FSRONLY && (fs->fs_magic == FS_UFS1_MAGIC || fs->fs_magic == FS_UFS2_MAGIC)) { + error = -1; /* fail on first error */ if (fs->fs_magic == FS_UFS2_MAGIC) { FCHK(fs->fs_sblockloc, !=, SBLOCK_UFS2, %#jx); } else if (fs->fs_magic == FS_UFS1_MAGIC) { FCHK(fs->fs_sblockloc, <, 0, %jd); FCHK(fs->fs_sblockloc, >, SBLOCK_UFS1, %jd); } + FCHK(fs->fs_sbsize, >, SBLOCKSIZE, %jd); + FCHK(fs->fs_sbsize, <, (signed)sizeof(struct fs), %jd); + FCHK(fs->fs_sbsize % fs->fs_fsize, !=, 0, %jd); FCHK(fs->fs_frag, <, 1, %jd); FCHK(fs->fs_frag, >, MAXFRAG, %jd); FCHK(fs->fs_bsize, <, MINBSIZE, %jd); @@ -438,12 +446,12 @@ validate_sblock(struct fs *fs, int flags) WCHK(fs->fs_old_rotdelay, !=, 0, %jd); WCHK(fs->fs_old_rps, !=, 60, %jd); WCHK(fs->fs_old_nspf, !=, fs->fs_fsize / sectorsize, %jd); - WCHK(fs->fs_old_cpg, !=, 1, %jd); + FCHK(fs->fs_old_cpg, !=, 1, %jd); WCHK(fs->fs_old_interleave, !=, 1, %jd); WCHK(fs->fs_old_trackskew, !=, 0, %jd); WCHK(fs->fs_old_cpc, !=, 0, %jd); WCHK(fs->fs_old_postblformat, !=, 1, %jd); - WCHK(fs->fs_old_nrpos, !=, 1, %jd); + FCHK(fs->fs_old_nrpos, !=, 1, %jd); WCHK(fs->fs_old_spc, !=, fs->fs_fpg * fs->fs_old_nspf, %jd); WCHK(fs->fs_old_nsect, !=, fs->fs_old_spc, %jd); WCHK(fs->fs_old_npsect, !=, fs->fs_old_spc, %jd); @@ -464,10 +472,18 @@ validate_sblock(struct fs *fs, int flags) FCHK(powerof2(fs->fs_fsize), ==, 0, %jd); FCHK(fs->fs_fpg, <, 3 * fs->fs_frag, %jd); FCHK(fs->fs_ncg, <, 1, %jd); - FCHK(fs->fs_ipg, <, 1, %jd); + FCHK(fs->fs_ipg, <, fs->fs_inopb, %jd); + FCHK(fs->fs_ipg % fs->fs_inopb, !=, 0, %jd); FCHK(fs->fs_ipg * fs->fs_ncg, >, (((int64_t)(1)) << 32) - INOPB(fs), %jd); + FCHK(fs->fs_cstotal.cs_nifree, <, 0, %jd); + FCHK(fs->fs_cstotal.cs_nifree, >, fs->fs_ipg * fs->fs_ncg, %jd); + FCHK(fs->fs_cstotal.cs_ndir, <, 0, %jd); + FCHK(fs->fs_cstotal.cs_ndir, >, + (fs->fs_ipg * fs->fs_ncg) - fs->fs_cstotal.cs_nifree, %jd); FCHK(fs->fs_sbsize, >, SBLOCKSIZE, %jd); + FCHK(fs->fs_sbsize, <, (signed)sizeof(struct fs), %jd); + FCHK(fs->fs_sbsize % fs->fs_fsize, !=, 0, %jd); FCHK(fs->fs_maxbsize, <, fs->fs_bsize, %jd); FCHK(powerof2(fs->fs_maxbsize), ==, 0, %jd); FCHK(fs->fs_maxbsize, >, FS_MAXCONTIG * fs->fs_bsize, %jd); @@ -482,6 +498,13 @@ validate_sblock(struct fs *fs, int flags) FCHK(fs->fs_old_cgoffset, <, 0, %jd); FCHK2(fs->fs_old_cgoffset, >, 0, ~fs->fs_old_cgmask, <, 0, %jd); FCHK(fs->fs_old_cgoffset * (~fs->fs_old_cgmask), >, fs->fs_fpg, %jd); + /* + * If anything has failed up to this point, it is usafe to proceed + * as checks below may divide by zero or make other fatal calculations. + * So if we have any errors at this point, give up. + */ + if (error) + return (error); FCHK(fs->fs_sblkno, !=, roundup( howmany(fs->fs_sblockloc + SBLOCKSIZE, fs->fs_fsize), fs->fs_frag), %jd); @@ -490,6 +513,8 @@ validate_sblock(struct fs *fs, int flags) FCHK(fs->fs_iblkno, !=, fs->fs_cblkno + fs->fs_frag, %jd); FCHK(fs->fs_dblkno, !=, fs->fs_iblkno + fs->fs_ipg / INOPF(fs), %jd); FCHK(fs->fs_cgsize, >, fs->fs_bsize, %jd); + FCHK(fs->fs_cgsize, <, fs->fs_fsize, %jd); + FCHK(fs->fs_cgsize % fs->fs_fsize, !=, 0, %jd); /* * This test is valid, however older versions of growfs failed * to correctly update fs_dsize so will fail this test. Thus we @@ -521,8 +546,8 @@ validate_sblock(struct fs *fs, int flags) * and ends in the data area of the same cylinder group. */ FCHK(fs->fs_size, <, 8 * fs->fs_frag, %jd); - WCHK(fs->fs_size, <=, (fs->fs_ncg - 1) * fs->fs_fpg, %jd); - WCHK(fs->fs_size, >, fs->fs_ncg * fs->fs_fpg, %jd); + FCHK(fs->fs_size, <=, (fs->fs_ncg - 1) * fs->fs_fpg, %jd); + FCHK(fs->fs_size, >, fs->fs_ncg * fs->fs_fpg, %jd); /* * If we are not requested to read in the csum data stop here * as the correctness of the remaining values is only important @@ -558,8 +583,8 @@ validate_sblock(struct fs *fs, int flags) */ WCHK(fs->fs_maxcontig, <, 0, %jd); WCHK(fs->fs_maxcontig, >, MAX(256, maxphys / fs->fs_bsize), %jd); - WCHK2(fs->fs_maxcontig, ==, 0, fs->fs_contigsumsize, !=, 0, %jd); - WCHK2(fs->fs_maxcontig, >, 1, fs->fs_contigsumsize, !=, + FCHK2(fs->fs_maxcontig, ==, 0, fs->fs_contigsumsize, !=, 0, %jd); + FCHK2(fs->fs_maxcontig, >, 1, fs->fs_contigsumsize, !=, MIN(fs->fs_maxcontig, FS_MAXCONTIG), %jd); return (error); } @@ -620,8 +645,8 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags, if (ffs_sbget(devfd, &protofs, UFS_STDSB, flags, filltype, readfunc) == 0) { if (msg) - printf("Attempted extraction of recovery data from " - "standard superblock: "); + printf("Attempt extraction of recovery data from " + "standard superblock.\n"); } else { /* * Final desperation is to see if alternate superblock @@ -630,7 +655,7 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags, if (msg) printf("Attempted extraction of recovery data from " "standard superblock: failed\nAttempt to find " - "boot zone recovery data: "); + "boot zone recovery data.\n"); /* * Look to see if recovery information has been saved. * If so we can generate a prototype superblock based @@ -675,13 +700,19 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags, /* * Scan looking for alternative superblocks. */ + flags = nocsum; + if (!msg) + flags |= UFS_NOMSG; for (cg = 0; cg < protofs->fs_ncg; cg++) { - sblk = dbtob(fsbtodb(protofs, cgsblock(protofs, cg))); - if (ffs_sbget(devfd, fsp, sblk, UFS_NOMSG | nocsum, filltype, + sblk = fsbtodb(protofs, cgsblock(protofs, cg)); + if (msg) + printf("Try cg %ld at sblock loc %jd\n", cg, + (intmax_t)sblk); + if (ffs_sbget(devfd, fsp, dbtob(sblk), flags, filltype, readfunc) == 0) { if (msg) - printf("succeeded with alternate superblock " - "at %jd\n", (intmax_t)btodb(sblk)); + printf("Succeeded with alternate superblock " + "at %jd\n", (intmax_t)sblk); UFS_FREE(protofs, filltype); return (0); } @@ -694,13 +725,18 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags, trynowarn: flags = UFS_NOWARNFAIL | UFS_NOMSG | nocsum; if (msg) { - printf("failed\n"); + printf("Finding an alternate superblock failed.\nCheck for " + "only non-critical errors in standard superblock\n"); flags &= ~UFS_NOMSG; } - if (ffs_sbget(devfd, fsp, UFS_STDSB, flags, filltype, readfunc) != 0) + if (ffs_sbget(devfd, fsp, UFS_STDSB, flags, filltype, readfunc) != 0) { + if (msg) + printf("Failed, superblock has critical errors\n"); return (ENOENT); + } if (msg) - printf("Using standard superblock with non-critical errors.\n"); + printf("Success, using standard superblock with " + "non-critical errors.\n"); return (0); }