Re: [PATCH v2] drm/ttm: return an error when kmapping a BO without pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ttm: NULL pointer dereference in CPU fault and memcpy fallbacks for SG BOs
--

commit ba111d71d680811150fde6d535fc740c46cc9d63
Author: Jesse Zhang <Jesse.Zhang@amd.com>

drm/ttm: return an error when kmapping a BO without pages

This commit adds checks to return an error when attempting to kmap or vmap a
buffer object that lacks a pages array, which occurs with imported SG tables.

> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index bd0b23ac2cc4b..f9458a4d5a09d 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -356,6 +356,10 @@ static int ttm_bo_kmap_ttm(struct ttm_buffer_object *bo,
>  	if (ret)
>  		return ret;
>  
> +	/* Imported sg tables only come with DMA addresses, no pages to map. */
> +	if (!ttm->pages)
> +		return -EINVAL;
> +

[Severity: High]
While this handles the kmap and vmap paths, can a similar NULL pointer
dereference occur in the CPU fault path and memcpy fallback for SG BOs?

If an imported BO is mapped via the DRM fd, userspace page faults can reach
ttm_bo_vm_fault_reserved(), which unconditionally dereferences ttm->pages:

drivers/gpu/drm/ttm/ttm_bo_vm.c:ttm_bo_vm_fault_reserved() {
    ...
    page = ttm->pages[page_offset];
    if (unlikely(!page && i == 0)) {
    ...
}

Similarly, if hardware copies fail on an imported BO and it falls back to
ttm_bo_move_memcpy(), the TT iterator initialized there is later used in
ttm_kmap_iter_tt_map_local(), which also accesses the pages array:

drivers/gpu/drm/ttm/ttm_bo_util.c:ttm_bo_move_memcpy() {
    ...
    dst_iter = ttm_kmap_iter_tt_init(&_dst_iter.tt, ttm);
    ...
}

drivers/gpu/drm/ttm/ttm_tt.c:ttm_kmap_iter_tt_map_local() {
    ...
    iosys_map_set_vaddr(dmap, kmap_local_page_prot(iter_tt->tt->pages[i],
                                                   iter_tt->prot));
    ...
}

Do these paths also need to check for a missing pages array and return an
error to prevent a crash?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009103419.4122940-1-Jesse.Zhang@amd.com?part=1