Got it. I will send a V2 patch to do a proper validation. On Mon, Jun 3, 2024 at 1:58 PM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > 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@xxxxxxxxxxxxx
