On Wed, Aug 31, 2022 at 08:16:39PM +0800, Guo Xuenan wrote: > For leaf dir, In most cases, there should be as many bestfree slots > as the dir data blocks that can fit under i_size (except for [1]). > > Root cause is we don't examin the number bestfree slots, when the slots > number less than dir data blocks, if we need to allocate new dir data > block and update the bestfree array, we will use the dir block number as > index to assign bestfree array, while we did not check the leaf buf > boundary which may cause UAF or other memory access problem. This issue > can also triggered with test cases xfs/473 from fstests. > > Considering the special case [1] , only add check bestfree array boundary, > to avoid UAF or slab-out-of bound. > > [1] https://lore.kernel.org/all/163961697197.3129691.1911552605195534271.stgit@magnolia/ > > Simplify the testcase xfs/473 with commands below: > DEV=/dev/sdb > MP=/mnt/sdb > WORKDIR=/mnt/sdb/341 #1. mkfs create new xfs image > mkfs.xfs -f ${DEV} > mount ${DEV} ${MP} > mkdir -p ${WORKDIR} > for i in `seq 1 341` #2. create leaf dir with 341 entries file name len 8 > do > touch ${WORKDIR}/$(printf "%08d" ${i}) > done > inode=$(ls -i ${MP} | cut -d' ' -f1) > umount ${MP} #3. xfs_db set bestcount to 0 > xfs_db -x ${DEV} -c "inode ${inode}" -c "dblock 8388608" \ > -c "write ltail.bestcount 0" > mount ${DEV} ${MP} > touch ${WORKDIR}/{1..100}.txt #4. touch new file, reproduce ..... > diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c > index d9b66306a9a7..4b2a72b3a6f3 100644 > --- a/fs/xfs/libxfs/xfs_dir2_leaf.c > +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c > @@ -819,6 +819,18 @@ xfs_dir2_leaf_addname( > */ > else > xfs_dir3_leaf_log_bests(args, lbp, use_block, use_block); > + /* > + * An abnormal corner case, bestfree count less than data > + * blocks, add a condition to avoid UAF or slab-out-of bound. > + */ > + if ((char *)(&bestsp[use_block]) > (char *)ltp) { > + xfs_trans_brelse(tp, lbp); > + if (tp->t_flags & XFS_TRANS_DIRTY) > + xfs_force_shutdown(tp->t_mountp, > + SHUTDOWN_CORRUPT_ONDISK); > + return -EFSCORRUPTED; > + } > + As I explained here: https://lore.kernel.org/linux-xfs/20220829081244.GT3600936@xxxxxxxxxxxxxxxxxxx/ We don't check for overruns caused by on-disk format corruptions in every operation - we catch the corruption when the metadata is first read into memory via the verifier. Please add a check for a corrupt/mismatched best sizes array to xfs_dir3_leaf_check_int() so that the corruption is detected on first read and the internal directory code is never exposed to such issues. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
