| Message ID | 20231204091649.3418987-1-zengchi@kylinos.cn |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix memory leaks in wave5_vpu_open_enc() and wave5_vpu_open_dec() | expand |
Hi, Le lundi 04 décembre 2023 à 17:16 +0800, Zeng Chi a écrit : > This patch fixes memory leaks on error escapes in wave5_vpu_open_enc() > and wave5_vpu_open_dec(). Please avoid sending twice the same patch. This is still a NAK. regards, Nicolas > > Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer") > Signed-off-by: Zeng Chi <zengchi@kylinos.cn> > --- > drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 +++-- > drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 5 +++-- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > index 8b1417ece96e..b0a045346bb7 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > @@ -1802,9 +1802,10 @@ static int wave5_vpu_open_dec(struct file *filp) > spin_lock_init(&inst->state_spinlock); > > inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); > - if (!inst->codec_info) > + if (!inst->codec_info) { > + kfree(inst); > return -ENOMEM; > - > + } > v4l2_fh_init(&inst->v4l2_fh, vdev); > filp->private_data = &inst->v4l2_fh; > v4l2_fh_add(&inst->v4l2_fh); > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > index f29cfa3af94a..bc94de9ea546 100644 > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > @@ -1546,9 +1546,10 @@ static int wave5_vpu_open_enc(struct file *filp) > inst->ops = &wave5_vpu_enc_inst_ops; > > inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); > - if (!inst->codec_info) > + if (!inst->codec_info) { > + kfree(inst); > return -ENOMEM; > - > + } > v4l2_fh_init(&inst->v4l2_fh, vdev); > filp->private_data = &inst->v4l2_fh; > v4l2_fh_add(&inst->v4l2_fh);
On 04/12/2023 13:55, Nicolas Dufresne wrote: > Hi, > > Le lundi 04 décembre 2023 à 17:16 +0800, Zeng Chi a écrit : >> This patch fixes memory leaks on error escapes in wave5_vpu_open_enc() >> and wave5_vpu_open_dec(). > Please avoid sending twice the same patch. This is still a NAK. tbf, this is a different patch, concerning the allocation of the codec_info within inst, not inst itself. > regards, > Nicolas > >> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer") >> Signed-off-by: Zeng Chi<zengchi@kylinos.cn> >> --- >> drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 +++-- >> drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 5 +++-- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c >> index 8b1417ece96e..b0a045346bb7 100644 >> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c >> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c >> @@ -1802,9 +1802,10 @@ static int wave5_vpu_open_dec(struct file *filp) >> spin_lock_init(&inst->state_spinlock); >> >> inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); >> - if (!inst->codec_info) >> + if (!inst->codec_info) { >> + kfree(inst); for consistency, would be better to jump to cleanup_inst. Also, maybe consider embedding codec_info in to struct vpu_instance to avoid the double alloc. I've not checked whether this is viable throughout the code, but from a quick scan of the original patch, it looks like it is always allocated and freed alongside inst. >> return -ENOMEM; >> - >> + } >> v4l2_fh_init(&inst->v4l2_fh, vdev); >> filp->private_data = &inst->v4l2_fh; >> v4l2_fh_add(&inst->v4l2_fh); >> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c >> index f29cfa3af94a..bc94de9ea546 100644 >> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c >> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c >> @@ -1546,9 +1546,10 @@ static int wave5_vpu_open_enc(struct file *filp) >> inst->ops = &wave5_vpu_enc_inst_ops; >> >> inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); >> - if (!inst->codec_info) >> + if (!inst->codec_info) { >> + kfree(inst); >> return -ENOMEM; >> - >> + } >> v4l2_fh_init(&inst->v4l2_fh, vdev); >> filp->private_data = &inst->v4l2_fh; >> v4l2_fh_add(&inst->v4l2_fh);
Le mardi 05 décembre 2023 à 17:36 +0000, Robert Beckett a écrit : > On 04/12/2023 13:55, Nicolas Dufresne wrote: > > Hi, > > > > Le lundi 04 décembre 2023 à 17:16 +0800, Zeng Chi a écrit : > > > This patch fixes memory leaks on error escapes in wave5_vpu_open_enc() > > > and wave5_vpu_open_dec(). > > Please avoid sending twice the same patch. This is still a NAK. > > tbf, this is a different patch, concerning the allocation of the > codec_info within inst, not inst itself. I've stopped after reading an identical subject line. I can apology for not noticing the difference, but I think an effort from the submitter could certainly help in the future. Nicolas > > > regards, > > Nicolas > > > > > Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer") > > > Signed-off-by: Zeng Chi<zengchi@kylinos.cn> > > > --- > > > drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 +++-- > > > drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 5 +++-- > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > > index 8b1417ece96e..b0a045346bb7 100644 > > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c > > > @@ -1802,9 +1802,10 @@ static int wave5_vpu_open_dec(struct file *filp) > > > spin_lock_init(&inst->state_spinlock); > > > > > > inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); > > > - if (!inst->codec_info) > > > + if (!inst->codec_info) { > > > + kfree(inst); > > for consistency, would be better to jump to cleanup_inst. > > Also, maybe consider embedding codec_info in to struct vpu_instance to > avoid the double alloc. I've not checked whether this is viable > throughout the code, but from a quick scan of the original patch, it > looks like it is always allocated and freed alongside inst. > > > > return -ENOMEM; > > > - > > > + } > > > v4l2_fh_init(&inst->v4l2_fh, vdev); > > > filp->private_data = &inst->v4l2_fh; > > > v4l2_fh_add(&inst->v4l2_fh); > > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > index f29cfa3af94a..bc94de9ea546 100644 > > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c > > > @@ -1546,9 +1546,10 @@ static int wave5_vpu_open_enc(struct file *filp) > > > inst->ops = &wave5_vpu_enc_inst_ops; > > > > > > inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); > > > - if (!inst->codec_info) > > > + if (!inst->codec_info) { > > > + kfree(inst); > > > return -ENOMEM; > > > - > > > + } > > > v4l2_fh_init(&inst->v4l2_fh, vdev); > > > filp->private_data = &inst->v4l2_fh; > > > v4l2_fh_add(&inst->v4l2_fh); >
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c index 8b1417ece96e..b0a045346bb7 100644 --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c @@ -1802,9 +1802,10 @@ static int wave5_vpu_open_dec(struct file *filp) spin_lock_init(&inst->state_spinlock); inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); - if (!inst->codec_info) + if (!inst->codec_info) { + kfree(inst); return -ENOMEM; - + } v4l2_fh_init(&inst->v4l2_fh, vdev); filp->private_data = &inst->v4l2_fh; v4l2_fh_add(&inst->v4l2_fh); diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c index f29cfa3af94a..bc94de9ea546 100644 --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c @@ -1546,9 +1546,10 @@ static int wave5_vpu_open_enc(struct file *filp) inst->ops = &wave5_vpu_enc_inst_ops; inst->codec_info = kzalloc(sizeof(*inst->codec_info), GFP_KERNEL); - if (!inst->codec_info) + if (!inst->codec_info) { + kfree(inst); return -ENOMEM; - + } v4l2_fh_init(&inst->v4l2_fh, vdev); filp->private_data = &inst->v4l2_fh; v4l2_fh_add(&inst->v4l2_fh);
This patch fixes memory leaks on error escapes in wave5_vpu_open_enc() and wave5_vpu_open_dec(). Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer") Signed-off-by: Zeng Chi <zengchi@kylinos.cn> --- drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 +++-- drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)