Hello! Thank you for promptly fixing this issue. May I ask if it's possible to assign a CVE to this vulnerability? Once again, thank you for your efforts in fixing this vulnerability! Wish you all the best. Tong 刘通 <lyutoon@xxxxxxxxx> 于2024年5月6日周一 20:24写道： > > Hello! > > Thank you for promptly fixing this issue. May I ask if it's possible to assign a CVE to this vulnerability? > Once again, thank you for your efforts in fixing this vulnerability! > > Wish you all the best. > > Tong > > Christoph Hellwig <hch@xxxxxx> 于2024年4月29日周一 14:15写道： >> >> xfs_bmapi_write can return 0 without actually returning a mapping in >> mval in two different cases: >> >> 1) when there is absolutely no space available to do an allocation >> 2) when converting delalloc space, and the allocation is so small >> that it only covers parts of the delalloc extent before the >> range requested by the caller >> >> Callers at best can handle one of these cases, but in many cases can't >> cope with either one. Switch xfs_bmapi_write to always return a >> mapping or return an error code instead. For case 1) above ENOSPC is >> the obvious choice which is very much what the callers expect anyway. >> For case 2) there is no really good error code, so pick a funky one >> from the SysV streams portfolio. >> >> This fixes the reproducer here: >> >> https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/ >> >> which uses reserved blocks to create file systems that are gravely >> out of space and thus cause at least xfs_file_alloc_space to hang >> and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc. >> >> Note that this patch does not actually make any caller but >> xfs_alloc_file_space deal intelligently with case 2) above. >> >> Signed-off-by: Christoph Hellwig <hch@xxxxxx> >> Reported-by: 刘通 <lyutoon@xxxxxxxxx> >> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> >> --- >> fs/xfs/libxfs/xfs_attr_remote.c | 1 - >> fs/xfs/libxfs/xfs_bmap.c | 46 ++++++++++++++++++++++++++------- >> fs/xfs/libxfs/xfs_da_btree.c | 20 ++++---------- >> fs/xfs/scrub/quota_repair.c | 6 ----- >> fs/xfs/scrub/rtbitmap_repair.c | 2 -- >> fs/xfs/xfs_bmap_util.c | 31 +++++++++++----------- >> fs/xfs/xfs_dquot.c | 1 - >> fs/xfs/xfs_iomap.c | 8 ------ >> fs/xfs/xfs_reflink.c | 14 ---------- >> fs/xfs/xfs_rtalloc.c | 2 -- >> 10 files changed, 57 insertions(+), 74 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >> index a8de9dc1e998a3..beb0efdd8f6b83 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.c >> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >> @@ -625,7 +625,6 @@ xfs_attr_rmtval_set_blk( >> if (error) >> return error; >> >> - ASSERT(nmap == 1); >> ASSERT((map->br_startblock != DELAYSTARTBLOCK) && >> (map->br_startblock != HOLESTARTBLOCK)); >> >> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c >> index 6053f5e5c71eec..f19191d6eade7e 100644 >> --- a/fs/xfs/libxfs/xfs_bmap.c >> +++ b/fs/xfs/libxfs/xfs_bmap.c >> @@ -4217,8 +4217,10 @@ xfs_bmapi_allocate( >> } else { >> error = xfs_bmap_alloc_userdata(bma); >> } >> - if (error || bma->blkno == NULLFSBLOCK) >> + if (error) >> return error; >> + if (bma->blkno == NULLFSBLOCK) >> + return -ENOSPC; >> >> if (bma->flags & XFS_BMAPI_ZERO) { >> error = xfs_zero_extent(bma->ip, bma->blkno, bma->length); >> @@ -4397,6 +4399,15 @@ xfs_bmapi_finish( >> * extent state if necessary. Details behaviour is controlled by the flags >> * parameter. Only allocates blocks from a single allocation group, to avoid >> * locking problems. >> + * >> + * Returns 0 on success and places the extent mappings in mval. nmaps is used >> + * as an input/output parameter where the caller specifies the maximum number >> + * of mappings that may be returned and xfs_bmapi_write passes back the number >> + * of mappings (including existing mappings) it found. >> + * >> + * Returns a negative error code on failure, including -ENOSPC when it could not >> + * allocate any blocks and -ENOSR when it did allocate blocks to convert a >> + * delalloc range, but those blocks were before the passed in range. >> */ >> int >> xfs_bmapi_write( >> @@ -4525,10 +4536,16 @@ xfs_bmapi_write( >> ASSERT(len > 0); >> ASSERT(bma.length > 0); >> error = xfs_bmapi_allocate(&bma); >> - if (error) >> + if (error) { >> + /* >> + * If we already allocated space in a previous >> + * iteration return what we go so far when >> + * running out of space. >> + */ >> + if (error == -ENOSPC && bma.nallocs) >> + break; >> goto error0; >> - if (bma.blkno == NULLFSBLOCK) >> - break; >> + } >> >> /* >> * If this is a CoW allocation, record the data in >> @@ -4566,7 +4583,6 @@ xfs_bmapi_write( >> if (!xfs_iext_next_extent(ifp, &bma.icur, &bma.got)) >> eof = true; >> } >> - *nmap = n; >> >> error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags, >> whichfork); >> @@ -4577,7 +4593,22 @@ xfs_bmapi_write( >> ifp->if_nextents > XFS_IFORK_MAXEXT(ip, whichfork)); >> xfs_bmapi_finish(&bma, whichfork, 0); >> xfs_bmap_validate_ret(orig_bno, orig_len, orig_flags, orig_mval, >> - orig_nmap, *nmap); >> + orig_nmap, n); >> + >> + /* >> + * When converting delayed allocations, xfs_bmapi_allocate ignores >> + * the passed in bno and always converts from the start of the found >> + * delalloc extent. >> + * >> + * To avoid a successful return with *nmap set to 0, return the magic >> + * -ENOSR error code for this particular case so that the caller can >> + * handle it. >> + */ >> + if (!n) { >> + ASSERT(bma.nallocs >= *nmap); >> + return -ENOSR; >> + } >> + *nmap = n; >> return 0; >> error0: >> xfs_bmapi_finish(&bma, whichfork, error); >> @@ -4684,9 +4715,6 @@ xfs_bmapi_convert_delalloc( >> if (error) >> goto out_finish; >> >> - error = -ENOSPC; >> - if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK)) >> - goto out_finish; >> if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock))) { >> xfs_bmap_mark_sick(ip, whichfork); >> error = -EFSCORRUPTED; >> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c >> index b13796629e2213..16a529a8878083 100644 >> --- a/fs/xfs/libxfs/xfs_da_btree.c >> +++ b/fs/xfs/libxfs/xfs_da_btree.c >> @@ -2297,8 +2297,8 @@ xfs_da_grow_inode_int( >> struct xfs_inode *dp = args->dp; >> int w = args->whichfork; >> xfs_rfsblock_t nblks = dp->i_nblocks; >> - struct xfs_bmbt_irec map, *mapp; >> - int nmap, error, got, i, mapi; >> + struct xfs_bmbt_irec map, *mapp = ↦ >> + int nmap, error, got, i, mapi = 1; >> >> /* >> * Find a spot in the file space to put the new block. >> @@ -2314,14 +2314,7 @@ xfs_da_grow_inode_int( >> error = xfs_bmapi_write(tp, dp, *bno, count, >> xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG, >> args->total, &map, &nmap); >> - if (error) >> - return error; >> - >> - ASSERT(nmap <= 1); >> - if (nmap == 1) { >> - mapp = ↦ >> - mapi = 1; >> - } else if (nmap == 0 && count > 1) { >> + if (error == -ENOSPC && count > 1) { >> xfs_fileoff_t b; >> int c; >> >> @@ -2339,16 +2332,13 @@ xfs_da_grow_inode_int( >> args->total, &mapp[mapi], &nmap); >> if (error) >> goto out_free_map; >> - if (nmap < 1) >> - break; >> mapi += nmap; >> b = mapp[mapi - 1].br_startoff + >> mapp[mapi - 1].br_blockcount; >> } >> - } else { >> - mapi = 0; >> - mapp = NULL; >> } >> + if (error) >> + goto out_free_map; >> >> /* >> * Count the blocks we got, make sure it matches the total. >> diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c >> index 0bab4c30cb85ab..90cd1512bba961 100644 >> --- a/fs/xfs/scrub/quota_repair.c >> +++ b/fs/xfs/scrub/quota_repair.c >> @@ -77,8 +77,6 @@ xrep_quota_item_fill_bmap_hole( >> irec, &nmaps); >> if (error) >> return error; >> - if (nmaps != 1) >> - return -ENOSPC; >> >> dq->q_blkno = XFS_FSB_TO_DADDR(mp, irec->br_startblock); >> >> @@ -444,10 +442,6 @@ xrep_quota_data_fork( >> XFS_BMAPI_CONVERT, 0, &nrec, &nmap); >> if (error) >> goto out; >> - if (nmap != 1) { >> - error = -ENOSPC; >> - goto out; >> - } >> ASSERT(nrec.br_startoff == irec.br_startoff); >> ASSERT(nrec.br_blockcount == irec.br_blockcount); >> >> diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c >> index 46f5d5f605c915..0fef98e9f83409 100644 >> --- a/fs/xfs/scrub/rtbitmap_repair.c >> +++ b/fs/xfs/scrub/rtbitmap_repair.c >> @@ -108,8 +108,6 @@ xrep_rtbitmap_data_mappings( >> 0, &map, &nmaps); >> if (error) >> return error; >> - if (nmaps != 1) >> - return -EFSCORRUPTED; >> >> /* Commit new extent and all deferred work. */ >> error = xrep_defer_finish(sc); >> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c >> index 53aa90a0ee3a85..2e6f08198c0719 100644 >> --- a/fs/xfs/xfs_bmap_util.c >> +++ b/fs/xfs/xfs_bmap_util.c >> @@ -721,33 +721,32 @@ xfs_alloc_file_space( >> if (error) >> goto error; >> >> - error = xfs_bmapi_write(tp, ip, startoffset_fsb, >> - allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp, >> - &nimaps); >> - if (error) >> - goto error; >> - >> - ip->i_diflags |= XFS_DIFLAG_PREALLOC; >> - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); >> - >> - error = xfs_trans_commit(tp); >> - xfs_iunlock(ip, XFS_ILOCK_EXCL); >> - if (error) >> - break; >> - >> /* >> * If the allocator cannot find a single free extent large >> * enough to cover the start block of the requested range, >> - * xfs_bmapi_write will return 0 but leave *nimaps set to 0. >> + * xfs_bmapi_write will return -ENOSR. >> * >> * In that case we simply need to keep looping with the same >> * startoffset_fsb so that one of the following allocations >> * will eventually reach the requested range. >> */ >> - if (nimaps) { >> + error = xfs_bmapi_write(tp, ip, startoffset_fsb, >> + allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp, >> + &nimaps); >> + if (error) { >> + if (error != -ENOSR) >> + goto error; >> + error = 0; >> + } else { >> startoffset_fsb += imapp->br_blockcount; >> allocatesize_fsb -= imapp->br_blockcount; >> } >> + >> + ip->i_diflags |= XFS_DIFLAG_PREALLOC; >> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); >> + >> + error = xfs_trans_commit(tp); >> + xfs_iunlock(ip, XFS_ILOCK_EXCL); >> } >> >> return error; >> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c >> index 13aba84bd64afb..43acb4f0d17433 100644 >> --- a/fs/xfs/xfs_dquot.c >> +++ b/fs/xfs/xfs_dquot.c >> @@ -357,7 +357,6 @@ xfs_dquot_disk_alloc( >> goto err_cancel; >> >> ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB); >> - ASSERT(nmaps == 1); >> ASSERT((map.br_startblock != DELAYSTARTBLOCK) && >> (map.br_startblock != HOLESTARTBLOCK)); >> >> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c >> index 9ce0f6b9df93e6..60463160820b62 100644 >> --- a/fs/xfs/xfs_iomap.c >> +++ b/fs/xfs/xfs_iomap.c >> @@ -322,14 +322,6 @@ xfs_iomap_write_direct( >> if (error) >> goto out_unlock; >> >> - /* >> - * Copy any maps to caller's array and return any error. >> - */ >> - if (nimaps == 0) { >> - error = -ENOSPC; >> - goto out_unlock; >> - } >> - >> if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) { >> xfs_bmap_mark_sick(ip, XFS_DATA_FORK); >> error = xfs_alert_fsblock_zero(ip, imap); >> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c >> index 7da0e8f961d351..5ecb52a234becc 100644 >> --- a/fs/xfs/xfs_reflink.c >> +++ b/fs/xfs/xfs_reflink.c >> @@ -430,13 +430,6 @@ xfs_reflink_fill_cow_hole( >> if (error) >> return error; >> >> - /* >> - * Allocation succeeded but the requested range was not even partially >> - * satisfied? Bail out! >> - */ >> - if (nimaps == 0) >> - return -ENOSPC; >> - >> convert: >> return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now); >> >> @@ -499,13 +492,6 @@ xfs_reflink_fill_delalloc( >> error = xfs_trans_commit(tp); >> if (error) >> return error; >> - >> - /* >> - * Allocation succeeded but the requested range was not even >> - * partially satisfied? Bail out! >> - */ >> - if (nimaps == 0) >> - return -ENOSPC; >> } while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff); >> >> return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now); >> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c >> index b476a876478d93..150f544445ca82 100644 >> --- a/fs/xfs/xfs_rtalloc.c >> +++ b/fs/xfs/xfs_rtalloc.c >> @@ -709,8 +709,6 @@ xfs_growfs_rt_alloc( >> nmap = 1; >> error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks, >> XFS_BMAPI_METADATA, 0, &map, &nmap); >> - if (!error && nmap < 1) >> - error = -ENOSPC; >> if (error) >> goto out_trans_cancel; >> /* >> -- >> 2.39.2 >>
