Re: [PATCH v3 1/5] gpu/buddy: Fix use-after-free in split_block() call sites

[Francois Dugast <francois.dugast@intel.com>]

On Mon, May 18, 2026 at 04:55:12PM +0100, Matthew Auld wrote:
> On 18/05/2026 15:14, Francois Dugast wrote:
> > When split_block() fails it returns before calling mark_split(), leaving
> > the block in the FREE state and still linked in the rbtree.  The four
> > err_undo paths then call __gpu_buddy_free() without first removing the
> > block from the tree, which leads to two distinct bugs:
> > 
> >   - If the buddy is also free, __gpu_buddy_free() merges the two siblings
> >     by calling gpu_block_free(mm, block) while block->rb is still linked
> >     in the tree.  Any subsequent rbtree traversal will follow the now-
> >     dangling pointer, causing a use-after-free.
> > 
> >   - In alloc_from_freetree(), where there is no buddy guard,
> >     __gpu_buddy_free() always reaches mark_free() -> rbtree_insert() with
> >     block still in the tree, corrupting the rbtree.
> > 
> > The same pattern is already used correctly in __force_merge(): call
> > rbtree_remove() to unlink the block before handing it to
> > __gpu_buddy_free().  Apply the same fix to all four err_undo sites.
> > 
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > Assisted-by: GitHub Copilot:claude-sonnet-4.6
> > ---
> >   drivers/gpu/buddy.c | 16 ++++++++++++----
> >   1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> > index eb1457376307..dac2027bb64a 100644
> > --- a/drivers/gpu/buddy.c
> > +++ b/drivers/gpu/buddy.c
> > @@ -737,8 +737,10 @@ __alloc_range_bias(struct gpu_buddy *mm,
> >   	buddy = __get_buddy(block);
> >   	if (buddy &&
> >   	    (gpu_buddy_block_is_free(block) &&
> > -	     gpu_buddy_block_is_free(buddy)))
> > +	     gpu_buddy_block_is_free(buddy))) {
> > +		rbtree_remove(mm, block);
> >   		__gpu_buddy_free(mm, block, false);
> > +	}
> >   	return ERR_PTR(err);
> >   }
> > @@ -847,8 +849,10 @@ alloc_from_freetree(struct gpu_buddy *mm,
> >   	return block;
> >   err_undo:
> > -	if (tmp != order)
> > +	if (tmp != order) {
> > +		rbtree_remove(mm, block);
> 
> Actually, I think this needs the same checking like elsewhere? Say we fail
> on the first split? Nothing was actually split, right?

I think this is unnecessary: for block this is tested above with
BUG_ON(!gpu_buddy_block_is_free(block)). If split_block() fails then it
happens before mark_split() so block remains free. If buddy is not free
then the merge loop is skipped in __gpu_buddy_free() but mark_free() is
called so we do remove + re-insert.

Also, the checks are added with patch #3 and the introduction of
__gpu_buddy_undo_splits().

Francois

> 
> >   		__gpu_buddy_free(mm, block, false);
> > +	}
> >   	return ERR_PTR(err);
> >   }
> > @@ -968,8 +972,10 @@ gpu_buddy_offset_aligned_allocation(struct gpu_buddy *mm,
> >   	buddy = __get_buddy(block);
> >   	if (buddy &&
> >   	    (gpu_buddy_block_is_free(block) &&
> > -	     gpu_buddy_block_is_free(buddy)))
> > +	     gpu_buddy_block_is_free(buddy))) {
> > +		rbtree_remove(mm, block);
> >   		__gpu_buddy_free(mm, block, false);
> > +	}
> >   	return ERR_PTR(err);
> >   }
> > @@ -1054,8 +1060,10 @@ static int __alloc_range(struct gpu_buddy *mm,
> >   	buddy = __get_buddy(block);
> >   	if (buddy &&
> >   	    (gpu_buddy_block_is_free(block) &&
> > -	     gpu_buddy_block_is_free(buddy)))
> > +	     gpu_buddy_block_is_free(buddy))) {
> > +		rbtree_remove(mm, block);
> >   		__gpu_buddy_free(mm, block, false);
> > +	}
> >   err_free:
> >   	if (err == -ENOSPC && total_allocated_on_err) {
>