On Wed, Feb 28, 2024 at 03:46:30PM -0800, Darrick J. Wong wrote: > If scrub (or the regular verifiers) hit anything, then we end up in > symlink_repair.c with CORRUPT set. In this case we set the target to > DUMMY_TARGET. Yes. > If the salvage functions recover fewer bytes than i_disk_size, then > we'll set the target to DUMMY_TARGET because that could lead to things > like: > > 0. touch autoexec autoexec@bat > 1. ln -s 'autoexec@bat' victimlink > 2. corrupt victimlink by s/@/\0/g' on the target > 3. repair salvages the target and ends up with 'autoexec' > > Alternately: > > 0. touch autoexec autoexec@bat > 1. ln -s 'autoexec@bat' victimlink > 2. corrupt victimlink by incrementing di_size (it's now 13) > 3. repair salvages the target and ends up with "autoexec@bat\0" > > In both of those cases, something's inconsistent between the buffer > contents and di_size. Yes. > There aren't supposed to be nulls in the target, > but whatever might have been in that byte originally is long gone. The > only thing to do here is replace it with DUMMY_TARGET. > > If salvage recovers more bytes than i_disk_size then we have no idea if > di_size was broken or not because the target isn't null-terminated. > In theory the kernel will never do this (because it zeroes the xfs_buf > contents in xfs_trans_buf_get) but fuzzers could do that. Now why do we even want to salvage parts of the symlink? A truncated symlink generally would cause more harm than just refusing to follow it.
