Re: [PATCH] dma-buf: don't WARN_ON_ONCE() on a failing dmabuf->ops->vmap()

[Christian König]

On 9/21/26 12:09, Janghyuck Kim wrote:
> dma_buf_vmap() escalates any non-zero return from dmabuf->ops->vmap()
> into a WARN_ON_ONCE(). But a failing vmap() is not, by itself,
> evidence of a violated invariant or a driver bug -- it's an ordinary
> runtime failure of a fallible operation, no different from any other
> allocation-shaped kernel API that can return an error. The dma-buf
> core has no visibility into what a given exporter's vmap()
> implementation considers a normal failure versus an internal bug;
> that distinction, if it matters at all, belongs to the exporter, not
> to this generic passthrough.

Well not quite, apart from some not so relevant HW workarounds the vmap callback is mostly used for fbdev emulation.

> -ENOMEM is a concrete, easily reproduced example of this. Page
> allocation on the vmap()/vmalloc() path can fail purely because the
> calling task has a fatal signal pending: should_reclaim_retry() in
> mm/page_alloc.c intentionally gives up reclaim and returns false
> once fatal_signal_pending(current) is true, so that a task that is
> already being killed (e.g. by a userspace low-memory killer) isn't
> stuck retrying reclaim on its way out. The resulting -ENOMEM from
> dmabuf->ops->vmap() is expected behaviour in that case, not a bug in
> the exporter -- and it is far from the only way ops->vmap() can
> legitimately fail (an exporter may just as well refuse to vmap a
> buffer for other, unrelated reasons of its own).
> 
> This is exactly the kind of condition Documentation/process/coding-style.rst
> warns against for WARN*():
> 
>     WARN*() must not be used for a condition that is expected to
>     trigger easily, for example, by user space actions.
> 
> A failing vmap() is reachable purely by driving the system into low
> memory and then issuing an ioctl that happens to map a dma-buf -- no
> driver misbehaviour required. Turning that into a WARN_ON_ONCE(),
> and on panic_on_warn kernels into an outright panic, doesn't help
> debugging; it just turns an otherwise-recoverable error into a fatal
> one.

WOW STOP! A driver allowing to vmap() through an IOCTL is a major no-go!

vmap() on a DMA-buf has very very limited use cases which should mostly only be trigger able as superuser.

At least on 32bit system you can otherwise trivially exhaust the vmap area of the kernel and that is even worse than a simple WARN_ON_ONCE().

Regards,
Christian.

> 
> Drop the WARN_ON_ONCE() and simply propagate the error, consistent
> with how other allocation-failure paths in the kernel are handled.
> The dma_buf_vmap_unlocked()/dma_buf_vmap() contract is unchanged:
> both still return the negative errno from dmabuf->ops->vmap() to the
> caller. Only the WARN_ON_ONCE() splat (and, transitively, the panic
> on panic_on_warn kernels) is removed.
> 
> Signed-off-by: Janghyuck Kim <janghyuck.kim@xxxxxxxxxxx>
> ---
>  drivers/dma-buf/dma-buf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 36cdc342627e..252495761a21 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1670,7 +1670,7 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map)
>         BUG_ON(iosys_map_is_set(&dmabuf->vmap_ptr));
> 
>         ret = dmabuf->ops->vmap(dmabuf, &ptr);
> -       if (WARN_ON_ONCE(ret))
> +       if (ret)
>                 return ret;
> 
>         dmabuf->vmap_ptr = ptr;
> --
> 2.34.1
>