git: 1a4c2eedad5b - main - cd9660: Fix dereferencing invalid RR outlen ptr
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 01 Nov 2026 04:02:33 UTC
The branch main has been updated by ngie:
URL: https://cgit.FreeBSD.org/src/commit/?id=1a4c2eedad5bd62c4ec3cdda276bf4579ecab6c6
commit 1a4c2eedad5bd62c4ec3cdda276bf4579ecab6c6
Author: Josh Megnauth <jo.sh@tutanota.com>
AuthorDate: 2026-10-05 19:40:15 +0000
Commit: Enji Cooper <ngie@FreeBSD.org>
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 <jo.sh@tutanota.com>
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 <bsd.test.mk>
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 <sys/types.h>
+#include <sys/stat.h>
+#include <sys/fs/cd9660/iso.h>
+#include <sys/fs/cd9660/cd9660_rrip.h>
+
+#include <err.h>
+#include <fcntl.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/* 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);
+}