* Re: [PATCH] drm/msm: avoid an unmatched fbdev IOVA pin
2026-11-09 17:01 [PATCH] drm/msm: avoid an unmatched fbdev IOVA pin Michael McAllister
@ 2026-11-09 17:25 ` sashiko-bot
2026-11-09 20:47 ` [PATCH v2] " Michael McAllister
2026-11-11 15:39 ` [PATCH] " Dmitry Baryshkov
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-09 17:25 UTC (permalink / raw)
To: Michael McAllister; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/msm: VMA close and vma_ref underflow in
msm_fbdev_driver_fbdev_probe()
--
commit a1a08825c16a09f8d3eedf5784d21f5c50d25613
Author: Michael McAllister <me@michaelmcallister.org>
drm/msm: avoid an unmatched fbdev IOVA pin
This commit replaces msm_gem_get_and_pin_iova() with msm_gem_get_iova() in
msm_fbdev_driver_fbdev_probe() and removes the corresponding
msm_gem_unpin_iova() call from the error path.
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index dd6d6c507d77e..c3bec6719ca04 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -137,12 +137,8 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> goto err_drm_gem_handle_delete;
> }
>
> - /*
> - * NOTE: if we can be guaranteed to be able to map buffer
> - * in panic (ie. lock-safe, etc) we could avoid pinning the
> - * buffer now:
> - */
> - ret = msm_gem_get_and_pin_iova(bo, priv->kms->vm, &paddr);
> + /* Scanout pins the buffer; only reserve its address here. */
> + ret = msm_gem_get_iova(bo, priv->kms->vm, &paddr);
> if (ret) {
> drm_err(dev, "failed to get buffer obj iova: %d\n", ret);
> goto err_drm_client_buffer_delete;
> @@ -162,7 +158,7 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> fbi->screen_buffer = msm_gem_get_vaddr(bo);
> if (IS_ERR(fbi->screen_buffer)) {
> ret = PTR_ERR(fbi->screen_buffer);
> - goto err_msm_gem_unpin;
> + goto err_msm_gem_vma_put;
> }
> fbi->screen_size = bo->size;
> fbi->fix.smem_start = paddr;
> @@ -179,8 +175,7 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>
> return 0;
>
> -err_msm_gem_unpin:
> - msm_gem_unpin_iova(bo, priv->kms->vm);
> +err_msm_gem_vma_put:
> msm_gem_vma_put(bo);
> err_drm_client_buffer_delete:
> drm_client_buffer_delete(buffer);
[Severity: Medium]
Does calling msm_gem_get_iova() without a matching msm_gem_vma_get() cause
the IOVA reservation to be closed immediately and underflow vma_ref on the
error path?
drm_gem_handle_create() takes the only vma_ref on bo via msm_gem_open(),
and neither drm_client_buffer_create() nor msm_gem_get_iova() increments
vma_ref. On the normal path, drm_gem_handle_delete() at the end of
msm_fbdev_driver_fbdev_probe() calls msm_gem_close() -> msm_gem_vma_put(),
which drops vma_ref to 0 and calls put_iova_spaces(..., true, "vma_put") to
close the VMA and release the IOVA range stored in fbi->fix.smem_start.
On the error path when msm_gem_get_vaddr() fails, err_msm_gem_vma_put calls
msm_gem_vma_put(bo) and then falls through to drm_gem_handle_delete(),
which calls msm_gem_vma_put(bo) a second time and underflows vma_ref to -1.
--
Sashiko AI review · https://sashiko.dev/#/patchset/179424371125.96483.2462884435294600442.msm-fbdev-pin-leak@michaelmcallister.org?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] drm/msm: avoid an unmatched fbdev IOVA pin
2026-11-09 17:01 [PATCH] drm/msm: avoid an unmatched fbdev IOVA pin Michael McAllister
2026-11-09 17:25 ` sashiko-bot
@ 2026-11-09 20:47 ` Michael McAllister
2026-11-11 15:39 ` [PATCH] " Dmitry Baryshkov
2 siblings, 0 replies; 4+ messages in thread
From: Michael McAllister @ 2026-11-09 20:47 UTC (permalink / raw)
To: robin.clark, lumag
Cc: abhinav.kumar, jesszhan0024, sean, marijn.suijten, airlied,
simona, linux-arm-msm, dri-devel, freedreno, linux-kernel
Unbinding the display on the Moto G10 produces a pin-count warning when
the console framebuffer is freed. The fbdev probe takes an IOVA pin,
but normal teardown never drops it.
Reserve the address with msm_gem_get_iova() instead. Scanout pins the
buffer while it is in use, and the CPU mapping holds its own pin.
Keep a VMA reference for fbdev so deleting the temporary GEM handle does
not release the reserved address. Drop that reference on probe failure
or when the framebuffer is destroyed. This also balances the existing
error-path VMA put, which otherwise drops the handle's reference twice.
Get the CPU mapping before publishing the framebuffer in the helper, so
a mapping failure does not leave helper->fb and helper->buffer pointing
to freed memory.
Fixes: 7ad0e8cf6317 ("drm/msm: Count how many times iova memory is pinned")
Signed-off-by: Michael McAllister <me@michaelmcallister.org>
---
Hi,
Changes in v2:
- Keep the fbdev VMA reference until teardown and balance the error paths.
- Get the CPU mapping before publishing the helper pointers.
Thanks,
Michael
drivers/gpu/drm/msm/msm_fbdev.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index dd6d6c507d..ed599884bf 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -48,6 +48,7 @@ static void msm_fbdev_fb_destroy(struct fb_info *info)
drm_fb_helper_fini(helper);
msm_gem_put_vaddr(bo);
+ msm_gem_vma_put(bo);
drm_client_buffer_delete(helper->buffer);
drm_client_release(&helper->client);
@@ -137,15 +138,18 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
goto err_drm_gem_handle_delete;
}
- /*
- * NOTE: if we can be guaranteed to be able to map buffer
- * in panic (ie. lock-safe, etc) we could avoid pinning the
- * buffer now:
- */
- ret = msm_gem_get_and_pin_iova(bo, priv->kms->vm, &paddr);
+ /* Keep the IOVA reserved after deleting the temporary GEM handle. */
+ msm_gem_vma_get(bo);
+ ret = msm_gem_get_iova(bo, priv->kms->vm, &paddr);
if (ret) {
drm_err(dev, "failed to get buffer obj iova: %d\n", ret);
- goto err_drm_client_buffer_delete;
+ goto err_msm_gem_vma_put;
+ }
+
+ fbi->screen_buffer = msm_gem_get_vaddr(bo);
+ if (IS_ERR(fbi->screen_buffer)) {
+ ret = PTR_ERR(fbi->screen_buffer);
+ goto err_msm_gem_vma_put;
}
DBG("fbi=%p, dev=%p", fbi, dev);
@@ -159,11 +163,6 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
drm_fb_helper_fill_info(fbi, helper, sizes);
- fbi->screen_buffer = msm_gem_get_vaddr(bo);
- if (IS_ERR(fbi->screen_buffer)) {
- ret = PTR_ERR(fbi->screen_buffer);
- goto err_msm_gem_unpin;
- }
fbi->screen_size = bo->size;
fbi->fix.smem_start = paddr;
fbi->fix.smem_len = bo->size;
@@ -179,10 +178,8 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
return 0;
-err_msm_gem_unpin:
- msm_gem_unpin_iova(bo, priv->kms->vm);
+err_msm_gem_vma_put:
msm_gem_vma_put(bo);
-err_drm_client_buffer_delete:
drm_client_buffer_delete(buffer);
err_drm_gem_handle_delete:
drm_gem_handle_delete(file, handle);
base-commit: 1e59855738454b14021b64115f0090eafc7526ae
--
2.39.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/msm: avoid an unmatched fbdev IOVA pin
2026-11-09 17:01 [PATCH] drm/msm: avoid an unmatched fbdev IOVA pin Michael McAllister
2026-11-09 17:25 ` sashiko-bot
2026-11-09 20:47 ` [PATCH v2] " Michael McAllister
@ 2026-11-11 15:39 ` Dmitry Baryshkov
2 siblings, 0 replies; 4+ messages in thread
From: Dmitry Baryshkov @ 2026-11-11 15:39 UTC (permalink / raw)
To: Michael McAllister
Cc: robin.clark, lumag, abhinav.kumar, jesszhan0024, sean,
marijn.suijten, airlied, simona, linux-arm-msm, dri-devel,
freedreno, linux-kernel
On Mon, Nov 09, 2026 at 05:01:51PM +0000, Michael McAllister wrote:
> Unbinding the display on the Moto G10 produces a pin-count warning when
> the console framebuffer is freed. The fbdev probe takes an IOVA pin,
> but normal teardown never drops it.
>
I'd say, it suggests that the teardown path must be fixed instead.
> Use msm_gem_get_iova() to reserve the address without taking that extra
> pin. Scanout already pins the buffer while it is in use, and the CPU
> mapping holds its own pin. Drop the matching unpin from the error path.
>
> Fixes: 7ad0e8cf6317 ("drm/msm: Count how many times iova memory is pinned")
> Signed-off-by: Michael McAllister <me@michaelmcallister.org>
> ---
> Hi,
>
> I found another pin-count warning while testing display teardown on the
> Moto G10. This one is in fbdev: it takes a pin when creating the console
> framebuffer, but doesn't drop it when the framebuffer is freed.
>
> I tested this alongside the other display fixes on 7.2.9, including
> unbind/rebind with the compositor stopped. It is independent of the DSI
> command-buffer fix I sent earlier.
>
> Thanks,
> Michael
>
> drivers/gpu/drm/msm/msm_fbdev.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index dd6d6c507d..c3bec6719c 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -137,12 +137,8 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> goto err_drm_gem_handle_delete;
> }
>
> - /*
> - * NOTE: if we can be guaranteed to be able to map buffer
> - * in panic (ie. lock-safe, etc) we could avoid pinning the
> - * buffer now:
> - */
> - ret = msm_gem_get_and_pin_iova(bo, priv->kms->vm, &paddr);
> + /* Scanout pins the buffer; only reserve its address here. */
> + ret = msm_gem_get_iova(bo, priv->kms->vm, &paddr);
> if (ret) {
> drm_err(dev, "failed to get buffer obj iova: %d\n", ret);
> goto err_drm_client_buffer_delete;
> @@ -162,7 +158,7 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> fbi->screen_buffer = msm_gem_get_vaddr(bo);
> if (IS_ERR(fbi->screen_buffer)) {
> ret = PTR_ERR(fbi->screen_buffer);
> - goto err_msm_gem_unpin;
> + goto err_msm_gem_vma_put;
> }
> fbi->screen_size = bo->size;
> fbi->fix.smem_start = paddr;
> @@ -179,8 +175,7 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>
> return 0;
>
> -err_msm_gem_unpin:
> - msm_gem_unpin_iova(bo, priv->kms->vm);
> +err_msm_gem_vma_put:
> msm_gem_vma_put(bo);
> err_drm_client_buffer_delete:
> drm_client_buffer_delete(buffer);
>
> base-commit: 1e59855738454b14021b64115f0090eafc7526ae
> --
> 2.39.5
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 4+ messages in thread