Re: [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> msm_framebuffer_prepare() bumps prepare_count before pinning, but returns
> straight out of the pin loop on error, leaking the count, the
> msm_gem_vma_get() reference and any planes already pinned.
> drm_atomic_helper_prepare_planes() does not call cleanup_fb() for the
> plane whose prepare_fb() failed, so nothing ever drops it.
> 
> Since commit 8ac37c88f991 ("drm/msm: Refcount framebuffer pins") a
> prepare which finds the count already non-zero returns early, assuming
> iova[] is populated.  With the count stuck, every later prepare of that
> framebuffer reports success while iova[] is still zero, and DPU scans out
> from a NULL base address:
> 
>   arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
>       iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
> 
> Unwind properly on failure instead.
> 
> Fixes: 8ac37c88f991 ("drm/msm: Refcount framebuffer pins")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

LLM:

Regression: writeback path double-decrements prepare_count

DPU writeback swallows the prepare error, so cleanup still runs for
a count that the error unwind has already dropped:

1. dpu_wb_conn_prepare_job() (dpu_writeback.c) calls
   dpu_encoder_prepare_wb_job(), which returns void, then always
   returns 0.
2. dpu_encoder_phys_wb_prepare_wb_job() calls
   msm_framebuffer_prepare(). On failure it logs and returns, so the
   error is lost.
3. drm_writeback_prepare_job() therefore sets job->prepared = true.
4. drm_writeback_cleanup_job() (drm_writeback.c:501) calls
   dpu_encoder_phys_wb_cleanup_wb_job(), which calls
   msm_framebuffer_cleanup().

With patch 2, the failed prepare has already dropped prepare_count
from 1 to 0. Cleanup decrements it again. Because prepare_count is
unsigned (patch 1), it wraps to UINT_MAX and never reaches zero, so
the planes are not unpinned. The next prepare of that framebuffer
does prepare_count++, sees a nonzero old value, and returns success
with iova[] still zero: the original NULL-base-address fault.

Before patch 2 the same path left the count at 1 and cleanup brought
it back to 0, so it was consistent.


Konrad:

I had a hunch to suggest using refcount_t in patch 1 and I think
we can go forward with that

Konrad