Re: [PATCH] xfs: don't walk off the end of a directory data block

[Dave Chinner <david@fromorbit.com>]

On Thu, May 30, 2024 at 11:10:57AM +0800, lei lu wrote:
> Thanks for your time.
> 
> I just add check for the fixed members because I see after the patch
> code there is some checks for dup and dep. "offset +
> be16_to_cpu(dup->length) > end" for dup and "offset +
> xfs_dir2_data_entsize(mp, dep->namelen) > end" for dep.
> “xfs_dir2_data_entsize(mp, dep->namelen)” ensures the alignment of the
> dep.

Sure, but go back and read what I said.

Detect the actual object corruption, not the downstream symptom.

IOWs, the verifier should be detecting the exact corruption you
induced.

Catching all the object corruptions prevents a buffer overrun.
We abort processing before we move beyond the end of the buffer.

IOWs, we need to:

1. verify dup->length is a multiple of XFS_DIR2_DATA_ALIGN; and
2. verify that if the last object in the buffer is less than
   xfs_dir2_data_entsize(mp, 1) bytes in size it must be a dup
   entry of exactly XFS_DIR2_DATA_ALIGN bytes in length.

If either of these checks fail, then the block is corrupt.
#1 will catch your induced corruption and fail immediately.
#2 will catch the runt entry in the structure without derefencing
past the end of the structure.

Can you now see how properly validating that the objects within the
structure will prevent buffer overruns from occurring without
needing generic buffer overrun checks?

-Dave.
-- 
Dave Chinner
david@fromorbit.com