Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Sep 21, 2026 at 10:22:38PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:35PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM points out that if an array element crosses a folio boundary,
> > xfile_get_folio returns a NULL folio pointer.  If this happens,
> > si->folio is also set to NULL, and calling folio_pos/folio_address will
> > just crash the kernel.  Teach this function to handle this condition by
> > falling back to reading the array element into scratchpad memory.
> > 
> > Cc: <stable@vger.kernel.org> # v6.6
> > Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> > index 2ce24bfe4c0fab..30a58e9d4378e4 100644
> > --- a/fs/xfs/scrub/xfarray.c
> > +++ b/fs/xfs/scrub/xfarray.c
> > @@ -830,22 +830,24 @@ xfarray_sort_scan(
> >  			return PTR_ERR(folio);
> >  		si->folio = folio;
> >  
> > -		si->first_folio_idx = xfarray_idx(si->array,
> > -				folio_pos(si->folio) + si->array->obj_size - 1);
> > +		if (si->folio) {
> > +			si->first_folio_idx = xfarray_idx(si->array,
> > +					folio_pos(si->folio) + si->array->obj_size - 1);
> 
> Overly long line.
> 
> > +			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
> 
> Another one.
> 
> > +	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {
> 
> And one more.

On second glance, I think a cleaner way to fix this is to move the logic
that loads the folio, computes the new {first,last}_folio_idx, and
validates them into a new helper:

static int
xfarray_sort_load_folio(
	struct xfarray_sortinfo	*si,
	xfarray_idx_t		idx,
	loff_t			idx_pos)
{
	struct folio		*folio;
	loff_t			next_pos;

	folio = xfile_get_folio(si->array->xfile, idx_pos, si->array->obj_size,
			XFILE_ALLOC);
	if (IS_ERR(folio))
		return PTR_ERR(folio);
	si->folio = folio;

	/* No folio?  Get the caller to read into the scratchpad. */
	if (!si->folio)
		return 0;

	si->first_folio_idx = xfarray_idx(si->array,
			folio_pos(si->folio) + si->array->obj_size - 1);

	next_pos = folio_next_pos(si->folio);
	si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
	if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
		si->last_folio_idx--;

	/*
	 * If this folio still doesn't cover the desired element, it must cross
	 * a folio boundary.  Get the caller to read into the scratchpad.
	 */
	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
		xfarray_sort_scan_done(si);
		return 0;
	}

	trace_xfarray_sort_scan(si, idx);
	return 0;
}

--D