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

[Frank Li]

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@xxxxxxxxx>
> > > ---
> > >  .../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@xxxxxxx>

Frank

>
> Best regards,
> Michail