Re: [PATCH] media: verisilicon: rockchip: Fix leaks in init

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

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

Le vendredi 21 août 2026 à 10:15 -0500, Frank Li a écrit :
> On Fri, Aug 21, 2026 at 12:16:23AM +0300, Michail Tatas wrote:
> > On Wed, Aug 19, 2026 at 12:56:40PM -0400, Frank Li wrote:
> > > On Wed, Aug 19, 2026 at 12:28:12PM +0300, Michail Tatas wrote:
> > > > if one of the dma_alloc_coherent in the init fucntion fails then
> > > > the previously allocated ones leak.
> > > > 
> > > > Fix by freeing them in the error path.
> > > > 
> > > > Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder")
> > > > Signed-off-by: Michail Tatas <michail.tatas@gmail.com>
> > > > ---
> > > >  .../verisilicon/rockchip_vpu981_hw_av1_dec.c  | 68 +++++++++++++++----
> > > >  1 file changed, 55 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> > > > index e4e21ad37323..fa77fd402412 100644
> > > > --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> > > > +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
> > > > @@ -369,6 +369,7 @@ void rockchip_vpu981_av1_dec_exit(struct hantro_ctx *ctx)
> > > > 
> > > >  int rockchip_vpu981_av1_dec_init(struct hantro_ctx *ctx)
> > > >  {
> > > > +	int ret = 0;
> > > >  	struct hantro_dev *vpu = ctx->dev;
> > > >  	struct hantro_av1_dec_hw_ctx *av1_dec = &ctx->av1_dec;
> > > > 
> > > > @@ -377,39 +378,54 @@ int rockchip_vpu981_av1_dec_init(struct hantro_ctx *ctx)
> > > >  	av1_dec->global_model.cpu = dma_alloc_coherent(vpu->dev, GLOBAL_MODEL_SIZE,
> > > >  						       &av1_dec->global_model.dma,
> > > >  						       GFP_KERNEL);
> > > > -	if (!av1_dec->global_model.cpu)
> > > > -		return -ENOMEM;
> > > > +	if (!av1_dec->global_model.cpu) {
> > > > +		ret = -ENOMEM;
> > > > +		goto global_model_cpu_err;
> > > > +	}
> > > > +
> > > >  	av1_dec->global_model.size = GLOBAL_MODEL_SIZE;
> > > > 
> > > >  	av1_dec->tile_info.cpu = dma_alloc_coherent(vpu->dev, AV1_TILE_INFO_SIZE,
> > > >  						    &av1_dec->tile_info.dma,
> > > >  						    GFP_KERNEL);
> > > > -	if (!av1_dec->tile_info.cpu)
> > > > -		return -ENOMEM;
> > > > +	if (!av1_dec->tile_info.cpu) {
> > > > +		ret = -ENOMEM;
> > > > +		goto tile_info_cpu_err;
> > > > +	}
> > > > +
> > > 
> > > This function is called by hantro_probe() if I am correct
> > > 
> > > there are dmam_alloc_coherent(), use dmam_alloc_coherent() will simple
> > > error handle and tear down.
> > > 
> > > Frank
> > 
> > Hello Frank,
> > 
> > I did not know about the dmam* functions thanks for letting me know.
> > I read some of the docs about them and they seem to not apply to
> > this particular case since this code is not called from the probe
> > function but rather the hantro_start_streaming function. Since
> > dmam_alloc_coherent() only releases memory at device removal,
> > it wouldn't free them when the stream ends and a new stream would
> > re-allocate while the old buffers lingered until detach.
> > So I think manual dma_free_coherent() error path is the correct
> > fit here.
> 
> Okay, make sense.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Applied, ty, and thanks for the review!

> 
> Frank
> 
> > 
> > Best regards,
> > Michail
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]