Re: [PATCH 1/2] isofs: validate directory records in isofs_read_level3_size()

[Matthias Goergens]

Hi Jan,

Thanks for looking at it.

I don't see how it can trigger there, but I'm probably missing
something, so here is my reasoning.  With 1/2 alone, offset is always
below bufsize at the top of the loop: a record that ends exactly at the
end of the block takes the existing "offset >= bufsize" branch further
down, which leaves offset & (bufsize - 1) == 0 and bumps block, so the
next iteration starts at offset 0 of the next block.  And
isofs_dir_record_valid() accepts a record that ends exactly at bufsize:
its test "len > bufsize - offset" is false when len equals the room
left.

I also tried it, running fs/isofs in a userspace harness on level 3
images where a record of a multi-extent file ends exactly at the end of
a block, including xorrisofs -iso-level 3 output with a sparse 8 GiB
file.  Neither 1/2 alone nor 1/2+2/2 rejects the directory, and the file
sizes come out right; a record whose length runs past the block is
rejected.

If you have a reproducer, or just a hint at the case you have in mind,
I'd be really happy to look at it.

Even if the code might be technically correct, it's confusing.  1/2 only
works because of a branch further down that 2/2 then deletes, so I'll
send a v2 that squashes the two: move on to the next block first, then
check the record, as you suggest, with the straddle code gone.  The end
result is the same code as after 2/2.

Matthias