[PATCH] drm/ttm: fix bulk_move corruption when adding a entry

[Christian König]

Am 21.06.23 um 17:42 schrieb Yunxiang Li:
> When the resource is the first in the bulk_move range, adding it again
> (thus moving it to the tail) will corrupt the list since the first
> pointer is not moved. This eventually lead to null pointer deref in
> ttm_lru_bulk_move_del()

Good catch.

>
> Fixes: fee2ede15542 ("drm/ttm: rework bulk move handling v5")
> Signed-off-by: Yunxiang Li <Yunxiang.Li at amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_resource.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 7333f7a87a2f..4b84cfb5f916 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -86,6 +86,9 @@ static void ttm_lru_bulk_move_pos_tail(struct ttm_lru_bulk_move_pos *pos,
>   				       struct ttm_resource *res)
>   {
>   	if (pos->last != res) {
> +		if (pos->first == res) {
> +			pos->first = list_next_entry(res, lru);
> +		}

Please drop the extra {}.

>   		list_move(&res->lru, &pos->last->lru);
>   		pos->last = res;
>   	}
> @@ -111,7 +114,9 @@ static void ttm_lru_bulk_move_del(struct ttm_lru_bulk_move *bulk,
>   {
>   	struct ttm_lru_bulk_move_pos *pos = ttm_lru_bulk_move_pos(bulk, res);
>   
> -	if (unlikely(pos->first == res && pos->last == res)) {
> +	if (unlikely(!pos->first)) {
> +		WARN(1, "ttm_lru_bulk_move_pos is corrupt");

Rather do a WARN_ON(!pos->first || !pos->last) before the ifs and change 
the last if to check for pos->last.

Regards,
Christian.

> +	} else if (unlikely(pos->first == res && pos->last == res)) {
>   		pos->first = NULL;
>   		pos->last = NULL;
>   	} else if (pos->first == res) {