[PATCH] isofs: advance to the next block when a record ends at the block end

[Matthias Goergens]

Since commit b2eb2e288604 ("isofs: Drop support of directory entries
straddling blocks") some ISO 9660 images list incomplete directories:
find returns fewer entries than before, and some of the names readdir
does return cannot be looked up.  The log fills with

  iso9660: Corrupted directory entry in block 0 of inode 55232

and on some images the walk reads a byte past the end of a buffer:

  BUG: KASAN: use-after-free in isofs_readdir+0x93b/0xc40

Affected are images with a directory record ending exactly at the end of
a logical block, which ECMA-119 allows and current tools produce.  On
the Debian 13.7.0 amd64 DVD-1, which has three, find returns 9163
entries instead of 9461:

  https://cdimage.debian.org/debian-cd/13.7.0/amd64/iso-dvd/debian-13.7.0-amd64-DVD-1.iso

The KASAN report is from a shareware CD with one in its root directory:

  https://archive.org/download/simtel-1-0295/SIMTEL1_0295.ISO

A local one, which lists 46 of its 65 files:

  mkdir tree
  for i in $(seq -f '%04g' 1 65); do echo hi > tree/FILE$i.TXT; done
  xorrisofs -iso-level 1 -o boundary.iso tree/

genisoimage pads the tail of a block instead of filling it exactly, so
its images have no such records.

That commit removed the reassembly of records crossing a block boundary.
Its condition was "offset >= bufsize", so it also handled a record
ending exactly at the end (offset == bufsize): drop the buffer,
increment the block, reset the offset.  Nothing does that now, so
do_isofs_readdir() and isofs_find_entry() read the next record's length
byte from bh->b_data + bufsize, one past the buffer.  Depending on that
byte the walk either treats the rest of the block as padding and skips
the next block of entries, or fails the record as corrupt and stops.

Straddling records stay rejected: isofs_dir_record_valid() fails them
because their length exceeds the remaining space, which is what that
commit relied on.  One ending exactly at the end fits in its block and
is accepted, so it is the advance, not the reassembly, that has to come
back.

Do it at the top of the loop: do_isofs_readdir() reaches the next
iteration from the multi-extent branch, "." and "..", the
hidden/associated skip (showassoc is off by default, so no mount option
is needed) and the fall-through, and all of them need covering.  The
record was validated to end within the block, so offset never exceeds
bufsize.

Fixes: b2eb2e288604 ("isofs: Drop support of directory entries straddling blocks")
Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
---
Not tagged for stable: b2eb2e288604 is in 7.3-rc1..rc3 and has not
appeared in a released kernel.

Validated under qemu with KASAN and UBSAN against the images above,
using a kernel built from the commit before b2eb2e288604 as the
reference for correct behaviour.

do_isofs_readdir(), isofs_find_entry() and isofs_read_level3_size() all
open-code this same block-end transition.  Happy to follow up with a
helper that shares it, kept out of here to keep the regression fix
minimal.

 fs/isofs/dir.c   | 12 ++++++++++++
 fs/isofs/namei.c | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c
index c7ca7603e97a..00a609c901dd 100644
--- a/fs/isofs/dir.c
+++ b/fs/isofs/dir.c
@@ -104,6 +104,18 @@ static int do_isofs_readdir(struct inode *inode, struct file *file,
 	while (ctx->pos < inode->i_size) {
 		int de_len;
 
+		/*
+		 * The previous record was validated to end within its
+		 * block; if it ended exactly at the end, the next record
+		 * starts at the beginning of the next block.
+		 */
+		if (offset == bufsize) {
+			brelse(bh);
+			bh = NULL;
+			block++;
+			offset = 0;
+		}
+
 		if (!bh) {
 			bh = isofs_bread(inode, block);
 			if (!bh)
diff --git a/fs/isofs/namei.c b/fs/isofs/namei.c
index 010682f5901a..5f5ed36d12e3 100644
--- a/fs/isofs/namei.c
+++ b/fs/isofs/namei.c
@@ -68,6 +68,18 @@ isofs_find_entry(struct inode *dir, struct dentry *dentry,
 		int de_len, match, i, dlen;
 		char *dpnt;
 
+		/*
+		 * The previous record was validated to end within its
+		 * block; if it ended exactly at the end, the next record
+		 * starts at the beginning of the next block.
+		 */
+		if (offset == bufsize) {
+			brelse(bh);
+			bh = NULL;
+			block++;
+			offset = 0;
+		}
+
 		if (!bh) {
 			bh = isofs_bread(dir, block);
 			if (!bh)
-- 
2.55.0