From nobody Sun Nov 01 04:02:33 2026 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 4jHLlL5tlSz6xXCg for ; Sun, 01 Nov 2026 04:02:38 +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) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 4jHLlL51qBz4ht2 for ; Sun, 01 Nov 2026 04:02:38 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793505758; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=COtv/PCnUgxl/58Uc7dNGKehKG+KuZqQxxxFlzq9XxM=; b=i5vqRYtBK/4VZPV0L0cSGCKrEjqqwhhDShbvMA/ECP/lUvuOAVl9YMWOJoW1hzD1NpE3z3 oc2b8nlYywAAgGiYtIgB9SdTfB0oISa7HK+ionKZm6FZXUbw8U7JW9lLCe3MMsGkNj6sKe 9qh0+3ZXAQPkP2nsbVXfgdIUZq/pSEPop1ugUcHYktILsKuLCqCC0TkdvjgZpIVpDmL0n2 f2xL5h+HOtCJ04J+Q7xo5gnZkgluNXKKHTh9OOR1Cddu2jyXRtbUoss0bHttWU+lNsaMEz Tkth3u8jPGyePjy8OXGQo0yUXZXh8JhW6xZenWh3Z+D60CM6jp2y00hpIjoTYA== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1793505758; b=bBGJbEfzuGSJoD8j1NBgTdYZv+pkt9lBe0K5va5VKWKj68UQXvq1GECUpTQgDW2mhfRYlw Jsx/Y2MaRivKVoha/vrr81AykvIMquANDg8UTLv1w/vzsFjwJDfWzyl1o8D60D0KjvuXKH C/6qBxkpkJ/9WxBFL0JUtWl97SwfyYaFd7GlFXFXKlmb/jQGBajquUPyXzSm9WmANX8L2X zm3NFs+TzAa0SmXjzqdxwEvJrngSVOWGLtJfgms3VOZ2o8HF3zvKCQwrV3QfZk/fb6nmJD zBQ8jMIU7CIKr4HEzdzgAXgxBTnwISU5dG35uSsf7uroMctiK8RsMpMk/LIQzQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1793505758; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=COtv/PCnUgxl/58Uc7dNGKehKG+KuZqQxxxFlzq9XxM=; b=RadRE6QcjG0YKMqp45mpxAKNE8434n8ozJWmjx//ufwR+eUbv12zXjdU5zysUFLBZzFzyR aU3ZQ2YAf13HTEqNq79+zBJidjZkZ7ZelPSKGP8ZXpobYpfDoHjavX4Twf+Mk+VepVV9Uw n8MNUk2TNEVZ2nPBQJsXUgLV1muIGAMMEcS31bCQTzczSN643P1Vxt3VIWqb5NgTu4iakL lucYp2TD/IKQVo+9FRcyhrRUahBTNadn1mfh0ZIWVATQSz5vW1olWFQGKR4yriZKhfsc2J aUpTgms72l5W76GFQFh9x/Kd2DKRRQrGcYt7RdPVlVXpaVCNaE//NvP+uzZLsQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4jHLlL4gXFzjt6 for ; Sun, 01 Nov 2026 04:02:38 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3c76c by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Sun, 01 Nov 2026 04:02:33 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Josh Megnauth From: Enji Cooper Subject: git: 1a4c2eedad5b - main - cd9660: Fix dereferencing invalid RR outlen ptr List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org List-Id: Commit messages for the main branch of the src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: ngie X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 1a4c2eedad5bd62c4ec3cdda276bf4579ecab6c6 Auto-Submitted: auto-generated Date: Sun, 01 Nov 2026 04:02:33 +0000 Message-Id: <6ae6b9d9.3c76c.623287c8@gitrepo.freebsd.org> The branch main has been updated by ngie: URL: https://cgit.FreeBSD.org/src/commit/?id=1a4c2eedad5bd62c4ec3cdda276bf4579ecab6c6 commit 1a4c2eedad5bd62c4ec3cdda276bf4579ecab6c6 Author: Josh Megnauth AuthorDate: 2026-10-05 19:40:15 +0000 Commit: Enji Cooper CommitDate: 2026-11-01 04:02:30 +0000 cd9660: Fix dereferencing invalid RR outlen ptr `ISO_RRIP_ANALYZE` is constructed in four places. Two of the initializers set the `outlen` field while the other two, `cd9660_rrip_analyze` and `cd9660_rrip_offset`, leave it uninitialized. The uninitialized paths can dereference an invalid pointer on error paths which causes a kernel panic. To fix, I zero out `ISO_RRIP_ANALYZE` in all four places where it's initialized and check that `outlen` is non-null on the error path in `cd9660_rrip_loop`. While testing the above, I encountered another kernel panic due to mounting continuing after the error branch. Evidently, the ISO continued mounting through the normal ISO 9660 path then triggered a panic down the line due to a very broken file system from the ISO. To fix that, I propagate the error from the RR path then fail in the mount path. A previous iteration of this commit included the ISO from the PR. The current iteration creates an ISO with makefs then corrupts it with a small helper written in C that is a condensed version of the CD 9660 code. It checks the input ISO for a a Rock Ridge extension reference then corrupts the length byte. This triggers the early failure in cd9660_rrip_offset. PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=296853 Signed-off-by: Josh Megnauth Reviewed-by: ngie Pull-Request: https://github.com/freebsd/freebsd-src/pull/2462 --- etc/mtree/BSD.tests.dist | 2 + sys/fs/cd9660/cd9660_rrip.c | 23 +++- sys/fs/cd9660/cd9660_vfsops.c | 9 +- sys/fs/cd9660/iso_rrip.h | 2 +- tests/sys/fs/Makefile | 1 + tests/sys/fs/cd9660/Makefile | 10 ++ tests/sys/fs/cd9660/cd9660_test.sh | 72 ++++++++++++ tests/sys/fs/cd9660/rrip_corrupt.c | 226 +++++++++++++++++++++++++++++++++++++ 8 files changed, 335 insertions(+), 10 deletions(-) diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist index 92a0008d2a0c..cec7a9f82627 100644 --- a/etc/mtree/BSD.tests.dist +++ b/etc/mtree/BSD.tests.dist @@ -828,6 +828,8 @@ file .. fs + cd9660 + .. fdescfs .. fusefs diff --git a/sys/fs/cd9660/cd9660_rrip.c b/sys/fs/cd9660/cd9660_rrip.c index 1f956434c642..c5588273c044 100644 --- a/sys/fs/cd9660/cd9660_rrip.c +++ b/sys/fs/cd9660/cd9660_rrip.c @@ -609,7 +609,8 @@ cd9660_rrip_loop(struct iso_directory_record *isodir, ISO_RRIP_ANALYZE *ana, } } if (ana->error) { - *ana->outlen = 0; + if (ana->outlen != NULL) + *ana->outlen = 0; result = 0; goto error; } @@ -680,7 +681,7 @@ int cd9660_rrip_analyze(struct iso_directory_record *isodir, struct iso_node *inop, struct iso_mnt *imp) { - ISO_RRIP_ANALYZE analyze; + ISO_RRIP_ANALYZE analyze = { 0 }; analyze.inop = inop; analyze.imp = imp; @@ -707,7 +708,7 @@ int cd9660_rrip_getname(struct iso_directory_record *isodir, char *outbuf, u_short *outlen, ino_t *inump, struct iso_mnt *imp) { - ISO_RRIP_ANALYZE analyze; + ISO_RRIP_ANALYZE analyze = { 0 }; RRIP_TABLE *tab; u_short c; @@ -747,7 +748,7 @@ int cd9660_rrip_getsymname(struct iso_directory_record *isodir, char *outbuf, u_short *outlen, struct iso_mnt *imp) { - ISO_RRIP_ANALYZE analyze; + ISO_RRIP_ANALYZE analyze = { 0 }; int ret; analyze.outbuf = outbuf; @@ -774,12 +775,14 @@ static RRIP_TABLE rrip_table_extref[] = { * Note: We insist on the ER field. */ int -cd9660_rrip_offset(struct iso_directory_record *isodir, struct iso_mnt *imp) +cd9660_rrip_offset(struct iso_directory_record *isodir, struct iso_mnt *imp, + int *errp) { ISO_RRIP_OFFSET *p; - ISO_RRIP_ANALYZE analyze; + ISO_RRIP_ANALYZE analyze = { 0 }; int ret; + *errp = 0; imp->rr_skip0 = 0; p = (ISO_RRIP_OFFSET *)(isodir->name + 1); if (memcmp(p, "SP\7\1\276\357", 6) != 0) { @@ -793,6 +796,14 @@ cd9660_rrip_offset(struct iso_directory_record *isodir, struct iso_mnt *imp) analyze.imp = imp; analyze.fields = ISO_SUSP_EXTREF; ret = cd9660_rrip_loop(isodir, &analyze, rrip_table_extref); + + /* + * Malformed RR; fail instead of allowing mount to continue (PR 296853). + */ + if (analyze.error) { + *errp = EINVAL; + return (-1); + } if ((ret & ISO_SUSP_EXTREF) == 0) return (-1); diff --git a/sys/fs/cd9660/cd9660_vfsops.c b/sys/fs/cd9660/cd9660_vfsops.c index 02d8acf6371a..0787e20f4b10 100644 --- a/sys/fs/cd9660/cd9660_vfsops.c +++ b/sys/fs/cd9660/cd9660_vfsops.c @@ -427,10 +427,13 @@ iso_mountfs(struct vnode *devvp, struct mount *mp) rootp = (struct iso_directory_record *)bp->b_data; - if ((isomp->rr_skip = cd9660_rrip_offset(rootp,isomp)) < 0) { - isomp->im_flags |= ISOFSMNT_NORRIP; + if ((isomp->rr_skip = cd9660_rrip_offset(rootp, isomp, + &error)) < 0) { + if (error != 0) + goto out; + isomp->im_flags |= ISOFSMNT_NORRIP; } else { - isomp->im_flags &= ~ISOFSMNT_GENS; + isomp->im_flags &= ~ISOFSMNT_GENS; } /* diff --git a/sys/fs/cd9660/iso_rrip.h b/sys/fs/cd9660/iso_rrip.h index 934a9f7bd2da..3586378944af 100644 --- a/sys/fs/cd9660/iso_rrip.h +++ b/sys/fs/cd9660/iso_rrip.h @@ -82,7 +82,7 @@ int cd9660_rrip_getsymname(struct iso_directory_record *isodir, char *outbuf, u_short *outlen, struct iso_mnt *imp); int cd9660_rrip_offset(struct iso_directory_record *isodir, - struct iso_mnt *imp); + struct iso_mnt *imp, int *errp); #endif /* _KERNEL */ #endif /* _ISOFS_CD9660_ISO_RRIP_H_ */ diff --git a/tests/sys/fs/Makefile b/tests/sys/fs/Makefile index 7c8194bb5f53..42fd087b16f6 100644 --- a/tests/sys/fs/Makefile +++ b/tests/sys/fs/Makefile @@ -7,6 +7,7 @@ TESTSDIR= ${TESTSBASE}/sys/fs TESTSRC= ${SRCTOP}/contrib/netbsd-tests/fs +TESTS_SUBDIRS+= cd9660 TESTS_SUBDIRS+= fdescfs #TESTS_SUBDIRS+= nullfs # XXX: needs rump # fusefs tests cannot be compiled/used without the googletest infrastructure. diff --git a/tests/sys/fs/cd9660/Makefile b/tests/sys/fs/cd9660/Makefile new file mode 100644 index 000000000000..e8e25e012c22 --- /dev/null +++ b/tests/sys/fs/cd9660/Makefile @@ -0,0 +1,10 @@ +PACKAGE= tests +TESTSDIR= ${TESTSBASE}/sys/fs/cd9660 + +BINDIR= ${TESTSDIR} +PROGS+= rrip_corrupt +CFLAGS+= -I${SRCTOP} + +ATF_TESTS_SH+= cd9660_test + +.include diff --git a/tests/sys/fs/cd9660/cd9660_test.sh b/tests/sys/fs/cd9660/cd9660_test.sh new file mode 100755 index 000000000000..d817b635d297 --- /dev/null +++ b/tests/sys/fs/cd9660/cd9660_test.sh @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: BSD-2-Clause + +# Create a Rock Ridge ISO that demonstrates its +# extensions, like longer dir and file names etc. +create_rock_ridge_iso() { + local iso_fs short long iso + + iso_fs="${1}" + short="${iso_fs}/short" + long="${short}/longernameabcd" + mkdir -p "${long}" || return 1 + + printf "test file" > "${iso_fs}/regular" || return 1 + printf "nested" > "${short}/nested" || return 1 + ln -s "regular" "${iso_fs}/symlinkfile" || return 1 + ln -s "short/longernameabcd" "${iso_fs}/symlinkdir" || return 1 + + iso="${2}" + makefs -t cd9660 -o rockridge "${iso}" "${iso_fs}" +} + +atf_test_case cd9660_rrip_pr296853 cleanup + +cd9660_rrip_pr296853_head() { + atf_set "descr" "Regression test for PR 296853" + atf_set "require.progs" "makefs mdconfig mount umount" + atf_set "require.user" "root" +} + +cd9660_rrip_pr296853_body() { + local iso_fs iso mnt md + + iso_fs="${PWD}/pr296853" + iso="${PWD}/malformed_pr296853.iso" + if ! create_rock_ridge_iso "${iso_fs}" "${iso}"; then + atf_fail "failed to create Rock Ridge ISO" + fi + atf_check "$(atf_get_srcdir)/rrip_corrupt" "${iso}" + + mnt="${PWD}/mnt" + atf_check mkdir "${mnt}" + atf_check -o save:mdconfig_pr296853 \ + mdconfig -a -t vnode -f "${iso}" + + # mdconfig_pr296853 contains the device name, e.g. md0. + md="$(cat mdconfig_pr296853)" + + # The invalid ISO should fail but not cause a kernel panic. + atf_check -s not-exit:0 -e match:"Invalid argument" \ + mount -t cd9660 "/dev/${md}" "${mnt}" +} + +cd9660_rrip_pr296853_cleanup() { + local iso_fs iso md mnt + + iso_fs="${PWD}/pr296853" + iso="${PWD}/malformed_pr296853.iso" + mnt="${PWD}/mnt" + umount -f "${mnt}" 2>/dev/null || true + + if md="$(cat mdconfig_pr296853 2>/dev/null)"; then + mdconfig -d -u "${md}" + rm -f mdconfig_pr296853 + fi + + rmdir "${mnt}" 2>/dev/null || true + rm -rf "${iso_fs}" "${iso}" +} + +atf_init_test_cases() { + atf_add_test_case cd9660_rrip_pr296853 +} diff --git a/tests/sys/fs/cd9660/rrip_corrupt.c b/tests/sys/fs/cd9660/rrip_corrupt.c new file mode 100644 index 000000000000..0c88105d270c --- /dev/null +++ b/tests/sys/fs/cd9660/rrip_corrupt.c @@ -0,0 +1,226 @@ +/*- + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +/* cd9660_vfsops.c scans descriptors starting at block 16. */ +static const size_t pvd_block = 16; + +/* + * ISO_DEFAULT_BLOCK_SIZE is 2048 bytes, so the PVD byte offset is the + * logical block number multiplied by the logical block size. + */ +static const off_t pvd_offset = (off_t)ISO_DEFAULT_BLOCK_SIZE * pvd_block; + +/* + * Corrupt a Rock Ridge ISO to demonstrate PR296853. + * + * The code is a condensed form of code from two files: + * - sys/fs/cd9660/cd9660_vfsops.c + * - sys/fs/cd9660/cd9660_rrip.c + * + * It finds the primary volume descriptor (PVD), then finds the root + * directory in the PVD, and finally walks the SUSP entries until it + * finds the Extensions Reference (ER). If the ER is Rock Ridge, it + * corrupts the length. + * + * The corrupted length triggers the bug in PR296853. + */ +static int +corrupt_rock_ridge(const char *path) +{ + const struct iso_directory_record *root, *root_entry; + const struct iso_primary_descriptor *pvd; + const struct iso_volume_descriptor *vdp; + uint8_t pvd_raw[ISO_DEFAULT_BLOCK_SIZE]; + uint8_t root_block[ISO_DEFAULT_BLOCK_SIZE]; + struct stat st; + off_t root_offset; + size_t name_len, record_len, su_len, su_offset; + ssize_t nread; + int fd; + uint32_t root_extent; + + int error = -1; + + fd = open(path, O_CLOEXEC | O_NOFOLLOW | O_RDWR); + if (fd == -1) { + warn("open %s", path); + goto out; + } + + /* Sanity check to ensure that the ISO is large enough to have a PVD. */ + if (fstat(fd, &st) == -1) { + warn("fstat"); + goto out; + } + if (st.st_size < pvd_offset + (off_t)sizeof(pvd_raw)) { + warnx("image is too small (no primary volume descriptor)"); + goto out; + } + + /* Read the PVD but don't retry short preads because it's a test. */ + nread = pread(fd, pvd_raw, sizeof(pvd_raw), pvd_offset); + if (nread == -1) { + warn("pread %s", path); + goto out; + } + if (nread != (ssize_t)sizeof(pvd_raw)) { + warnx("short read %s (%zd != %zu)", path, nread, + sizeof(pvd_raw)); + goto out; + } + + /* + * Check that it's actually the primary descriptor. + * Logic is from iso_mountfs in cd9660_vfsops.c. + */ + vdp = (const struct iso_volume_descriptor *)pvd_raw; + if (memcmp(vdp->id, ISO_STANDARD_ID, sizeof(vdp->id)) != 0) { + warnx("read offset is not a volume descriptor %s", path); + goto out; + } + if (isonum_711(vdp->type) != ISO_VD_PRIMARY) { + warnx("primary volume descriptor isn't at expected location %s", + path); + goto out; + } + pvd = (const struct iso_primary_descriptor *)pvd_raw; + root = (const struct iso_directory_record *)pvd->root_directory_record; + + /* + * Now that we have the PVD, find the root directory so that we can + * mangle the system use area. + */ + root_extent = isonum_733(root->extent); + root_offset = ((off_t)root_extent + isonum_711(root->ext_attr_length)) * + ISO_DEFAULT_BLOCK_SIZE; + if (root_offset > st.st_size || + (off_t)sizeof(root_block) > st.st_size - root_offset) { + warnx("root directory block is outside of image %s", path); + goto out; + } + + nread = pread(fd, root_block, sizeof(root_block), root_offset); + if (nread == -1) { + warn("pread %s", path); + goto out; + } + if (nread != (ssize_t)sizeof(root_block)) { + warnx("short read %s (%zd != %zu)", path, nread, + sizeof(root_block)); + goto out; + } + + /* Logic here is a condensed cd9660_rrip_loop */ + root_entry = (const struct iso_directory_record *)root_block; + record_len = isonum_711(root_entry->length); + name_len = isonum_711(root_entry->name_len); + + su_offset = ISO_DIRECTORY_RECORD_SIZE + name_len; + /* Even name len has one byte padding; see cd9660_rrip_loop. */ + if ((name_len & 1) == 0) + su_offset++; + su_len = record_len - su_offset; + + for (size_t susp_offset = 0; susp_offset < su_len;) { + const ISO_SUSP_HEADER *header; + size_t entry_len; + + if (su_len - susp_offset < sizeof(*header)) { + warnx("SUSP header too small %s", path); + goto out; + } + header = (const ISO_SUSP_HEADER *)(root_block + su_offset + + susp_offset); + entry_len = isonum_711(header->length); + + if (entry_len < sizeof(*header) || + entry_len > su_len - susp_offset) { + warnx("invalid SUSP entry length %s", path); + goto out; + } + + /* + * Corrupt ISO_RRIP_EXTREF in the Extensions Reference (ER). + * See: cd9660_rrip_loop and cd9660_rrip_extref. + */ + if (isonum_711(header->version) == 1 && + memcmp(header->type, "ER", 2) == 0) { + off_t offset; + const ISO_RRIP_EXTREF *extref; + uint8_t bad_len = 4; + + /* Expected Rock Ridge but got something else. */ + if (entry_len < sizeof(ISO_RRIP_EXTREF)) { + warnx( + "ER too short for Rock Ridge %s (%zu < %zu)", + path, entry_len, sizeof(ISO_RRIP_EXTREF)); + goto out; + } + + extref = (const ISO_RRIP_EXTREF *)header; + if (isonum_711(extref->version) != 1) { + susp_offset += entry_len; + continue; + } + + offset = root_offset + su_offset + susp_offset + + offsetof(ISO_SUSP_HEADER, length); + + /* + * Corrupt the Rock Ridge length. + * This triggers the bug in PR296853 because the initial + * parse of RR fails. At that point, the outlen field is + * arbitrary bytes (NULL in the fix) because that's not + * one of the paths that write a buffer, which would + * require outlen. + */ + if (pwrite(fd, &bad_len, sizeof(bad_len), offset) != + sizeof(bad_len)) { + warn("pwrite to corrupt ISO %s", path); + goto out; + } + + error = 0; + goto out; + } + + susp_offset += entry_len; + } + + warnx("did not find a valid ER entry in %s", path); +out: + if (fd != -1) + (void)close(fd); + return (error); +} + +int +main(int argc, char *argv[]) +{ + if (argc != 2) { + fprintf(stderr, "usage: %s image\n", argv[0]); + exit(EXIT_FAILURE); + } + + if (corrupt_rock_ridge(argv[1]) == -1) { + errx(EXIT_FAILURE, "failed to corrupt Rock Ridge ISO: %s", + argv[1]); + } + + return (EXIT_SUCCESS); +}