Re: [PATCH] isofs: bound Rock Ridge symlink components to the SL record

From: Bryam Vargas

Date: Sat Jun 06 2026 - 21:18:58 EST


On Sat, 6 Jun 2026, Michael Bommarito <michael.bommarito@xxxxxxxxx> wrote:
> one note I'd mention is that I returned NULL here instead of breaking so
> that readlink() would fail with -EIO downstream. Maybe I'm missing
> something elsewhere, but I think this design results in silent truncation
> and a potentially confused caller who thinks the symlink was successful.

Agreed. In get_symlink_chunk() a break leaves rock_ridge_symlink_read_folio()
with rpnt != link, which it treats as success, so readlink() returns the
truncated target. v2 returns NULL there instead -- matching the existing
plimit-overflow checks in the same function -- so a malformed SL record now
fails with -EIO rather than silently truncating.

I left the break in parse_rock_ridge_inode_internal(): that site only bounds
the i_size walk, and the link is read back through get_symlink_chunk(), which
now errors on the same record, so the truncated i_size is never consumed. I'm
happy to make that site a hard error too if you'd rather both reject.

I checked the three behaviours on the exact loop, with one valid component
followed by an over-long one: unpatched -> 255-byte out-of-bounds read; break
-> readlink() returns the truncated "abc/"; return NULL -> -EIO; a well-formed
link is unaffected.

v2 below, with your suggestion.

Bryam