| Message ID | GV1P250MB0737D9E8D64211A49774E5828F3E2@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 2f59648aed8ba538e2ff3cd7edcb85f4501faa25 |
| Headers | show |
| Series | [FFmpeg-devel,1/7] avcodec/wavpack: Fix leak and segfault on reallocation error | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Andreas Rheinhardt: > av_realloc_f() frees the buffer it is given on allocation > failure. But in this case, the buffer is an array of > ownership pointers, causing leaks on error. Furthermore, > the count of pointers is unchanged on error and the codec's > close function uses it to free said ownership pointers, > causing a NPD. > This is a regression since 46412a8935e4632b2460988bfce4152c7dccce22. > > Fix this by switching to av_realloc_array(). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Actually, one only needs one WavpackFrameContext at a time, given > that this decoder does not do proper slice threading. > Alternatively, one could implement proper slice threading. > > libavcodec/wavpack.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c > index 7e60a1456a..36bd4662e8 100644 > --- a/libavcodec/wavpack.c > +++ b/libavcodec/wavpack.c > @@ -973,9 +973,11 @@ static inline int wv_unpack_mono(WavpackFrameContext *s, GetBitContext *gb, > > static av_cold int wv_alloc_frame_context(WavpackContext *c) > { > - c->fdec = av_realloc_f(c->fdec, c->fdec_num + 1, sizeof(*c->fdec)); > - if (!c->fdec) > + WavpackFrameContext **fdec = av_realloc_array(c->fdec, c->fdec_num + 1, sizeof(*c->fdec)); > + > + if (!fdec) > return -1; > + c->fdec = fdec; > > c->fdec[c->fdec_num] = av_mallocz(sizeof(**c->fdec)); > if (!c->fdec[c->fdec_num]) Will apply this patchset tomorrow unless there are objections. - Andreas
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c index 7e60a1456a..36bd4662e8 100644 --- a/libavcodec/wavpack.c +++ b/libavcodec/wavpack.c @@ -973,9 +973,11 @@ static inline int wv_unpack_mono(WavpackFrameContext *s, GetBitContext *gb, static av_cold int wv_alloc_frame_context(WavpackContext *c) { - c->fdec = av_realloc_f(c->fdec, c->fdec_num + 1, sizeof(*c->fdec)); - if (!c->fdec) + WavpackFrameContext **fdec = av_realloc_array(c->fdec, c->fdec_num + 1, sizeof(*c->fdec)); + + if (!fdec) return -1; + c->fdec = fdec; c->fdec[c->fdec_num] = av_mallocz(sizeof(**c->fdec)); if (!c->fdec[c->fdec_num])
av_realloc_f() frees the buffer it is given on allocation failure. But in this case, the buffer is an array of ownership pointers, causing leaks on error. Furthermore, the count of pointers is unchanged on error and the codec's close function uses it to free said ownership pointers, causing a NPD. This is a regression since 46412a8935e4632b2460988bfce4152c7dccce22. Fix this by switching to av_realloc_array(). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Actually, one only needs one WavpackFrameContext at a time, given that this decoder does not do proper slice threading. Alternatively, one could implement proper slice threading. libavcodec/wavpack.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)