Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] A defragmentation move that requires an intermediate bounce buffer (returns -EMULTIHOP) permanently leaks the original ttm_tt, loses user data, and exposes uninitialized memory. --
--- Patch [5]: [PATCH v2 05/33] drm/ttm: Support defragmentation moves --- commit 4cef86e1c4adcce1b49f5be21dce811803bbfbeb Author: Matthew Brost <[email protected]> drm/ttm: Support defragmentation moves Add infrastructure to let a driver re-back an already-resident BO with fresh pages (e.g. at a more beneficial order) by reusing the normal move machinery. > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 3980f376e3ba4..d05da4eb1d8a4 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c [ ... ] > @@ -125,36 +125,93 @@ static int ttm_bo_handle_move_mem(struct > ttm_buffer_object *bo, [ ... ] > + if (defrag) > + ttm_bo_unmap_virtual(bo); > + > ret = bdev->funcs->move(bo, evict, ctx, mem, hop); > if (ret) { > if (ret == -EMULTIHOP) > return ret; [Severity: Critical] Does returning here directly skip restoring the original backing? If bdev->funcs->move() returns -EMULTIHOP during a defragmentation move, we return immediately instead of going to out_err. This leaves bo->ttm pointing to the new unpopulated TT, and bo->defrag_old_tt holding the original populated TT. When ttm_bo_validate() subsequently handles the multi-hop by calling ttm_bo_bounce_temp_buffer(), it passes the same ctx where ctx->defrag is still true. Will the nested ttm_bo_handle_move_mem() misclassify this bounce as another defrag move and stash the empty bo->ttm into bo->defrag_old_tt? This appears to overwrite the original TT, permanently leaking it and resulting in the loss of user data while exposing uninitialized memory. > goto out_err; > } > > + if (bo->defrag_old_tt) { > + ttm_tt_unpopulate(bo->bdev, bo->defrag_old_tt); > + ttm_tt_destroy(bo->bdev, bo->defrag_old_tt); > + bo->defrag_old_tt = NULL; > + } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
