Hi, Dave, Do you have further comments and/or suggestions? Thanks, LL On Mon, Jun 3, 2024 at 4:02 PM lei lu <llfamsec@xxxxxxxxx> wrote: > > This adds sanity checks for xfs_dir2_data_unused and xfs_dir2_data_entry > to make sure don'y stray beyond valid memory region. Before patching, the > loop simply checks that the start offset of the dup and dep is within the > range. So in a crafted image, if last entry is xfs_dir2_data_unused, we > can change dup->length to dup->length-1 and leave 1 byte of space. In the > next traversal, this space will be considered as dup or dep. We may > encounter an out of bound read when accessing the fixed members. > > In the patch, we check dup->length % XFS_DIR2_DATA_ALIGN != 0 to make > sure that dup is 8 byte aligned. And we also check the size of each entry > is greater than xfs_dir2_data_entsize(mp, 1) which ensures that there is > sufficient space to access fixed members. It should be noted 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. > > Signed-off-by: lei lu <llfamsec@xxxxxxxxx> > --- > fs/xfs/libxfs/xfs_dir2_data.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c > index dbcf58979a59..dd6d43cdf0c5 100644 > --- a/fs/xfs/libxfs/xfs_dir2_data.c > +++ b/fs/xfs/libxfs/xfs_dir2_data.c > @@ -178,6 +178,11 @@ __xfs_dir3_data_check( > struct xfs_dir2_data_unused *dup = bp->b_addr + offset; > struct xfs_dir2_data_entry *dep = bp->b_addr + offset; > > + if (offset > end - xfs_dir2_data_entsize(mp, 1)) > + if (end - offset != XFS_DIR2_DATA_ALIGN || > + be16_to_cpu(dup->freetag) != XFS_DIR2_DATA_FREE_TAG) > + return __this_address; > + > /* > * If it's unused, look for the space in the bestfree table. > * If we find it, account for that, else make sure it > @@ -188,6 +193,8 @@ __xfs_dir3_data_check( > > if (lastfree != 0) > return __this_address; > + if (be16_to_cpu(dup->length) % XFS_DIR2_DATA_ALIGN != 0) > + return __this_address; > if (offset + be16_to_cpu(dup->length) > end) > return __this_address; > if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) != > -- > 2.34.1 >
