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

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

[Janghyuck Kim]

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.

-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.

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@samsung.com>
---
 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

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@samsung.com>
> ---
>  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
> 

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

[Janghyuck Kim]

[-- Attachment #1: Type: text/plain, Size: 4951 bytes --]

On 2026-09-21 13:39, Christian König wrote:
> 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.
> 
Fair point about fbdev being the historical motivation. However,
->vmap() is not limited to fbdev; for example, the mainline CMA dma-buf
heap calls vmap() and explicitly returns -ENOMEM when the mapping cannot
be established.

More generally, ->vmap() is a fallible callback whose API allows it to
return an error. The exact reason for failure is exporter-specific, so I
don't think the dma-buf core should treat every ->vmap() failure as an
exporter bug and unconditionally WARN on it.

The fatal_signal_pending() case is simply one concrete example of how
vmap() can fail under memory pressure without indicating a driver bug.
> > -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().
Fair point. To be clear, I'm not arguing that an unprivileged ioctl
should be allowed to trigger vmap() without appropriate access control.
If a driver exposes that, I agree that it should be addressed in the
driver.

My point is separate: even a legitimate, one-off vmap() may fail under
memory pressure, and such a failure does not by itself indicate driver
misbehaviour. The fatal_signal_pending() case is one example where
memory allocation may give up rather than continue reclaim.

I'll drop the "no driver misbehaviour required" wording, since I agree
it could be read as endorsing unrestricted ioctl-triggered vmap, which
wasn't my intention.
> 
> 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@samsung.com>
> > ---
> >  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
> > 
> 
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]