On Wed, Feb 28, 2024 at 10:53:18AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 28, 2024 at 10:37:40AM -0800, Darrick J. Wong wrote:
> > Going back to [1] from last year, I finally /did/ find a magic symlink
> > target that actually does trip EIO. That solution is to set the buffer
> > contents to a string that is so long that it exceeds NAME_MAX.
> > Userspace can readlink this string, but it will never resolve anywhere
> > in the directory tree.
> >
> > What if this unconditionally set the link target to DUMMY_TARGET instead
> > of salvaging partial targets?
>
> Sounds good to me.
I overlooked something this morning -- if the caller passes in
XFS_SCRUB_IFLAG_FORCE_REBUILD, that might be the free space defragmenter
trying to get us to move the remote target block somewhere else. For
that usecase, if the symlink scrub doesn't find any problems and we read
in exactly i_size bytes, I think we want to write that back to the
symlink, and not the DUMMY_TARGET.
Something like:
if (FORCE_REBUILD && !CORRUPT) {
if (sc->ip->i_df.if_format == XFS_DINODE_FMT_LOCAL)
ret = xrep_symlink_salvage_inline(sc);
else
ret = xrep_symlink_salvage_remote(sc);
if (ret < 0)
return ret;
if (ret != ip->i_disk_size)
ret = 0;
}
target_buf[ret] = 0;
/*
* Change an empty target into a dummy target and clear the symlink
* target zapped flag.
*/
if (target_buf[0] == 0) {
sc->sick_mask |= XFS_SICK_INO_SYMLINK_ZAPPED;
sprintf(target_buf, DUMMY_TARGET);
}
Can we allow that without risking truncation making the symlink point to
some unintended place?
--D
