| Message ID | 20210124213253.1344753-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2bcec40ccee6a848db4034c4d096a44bf10db957 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avfilter/vf_nnedi: Fix segfault when prescreening is disabled | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
LGTM On Sun, Jan 24, 2021 at 10:33 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Since c737f6edcef74a64f4d0ebcefa970bd31266d512 prescreening is > nevertheless run because of a wrong check: "if (s->prescreen > 0)". > s->prescreen is an array of two function pointers that is contained in > the context and comparing it with 0 (i.e. NULL) is actually undefined > behaviour, because NULL and s->prescreen do not point to the same > object (NULL after all never points to any object). Nevertheless both > Clang as well as GCC compile this to code that treat s->prescreen > 0 as > true, leading to segfaults, because the code then tries to access the > -1th member of an array. > > This commit fixes the check as well as another such check a few lines > below. > > (Found via compiler warnings enabled by -pedantic: > "ordered comparison between pointer and zero is an extension".) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavfilter/vf_nnedi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/vf_nnedi.c b/libavfilter/vf_nnedi.c > index 1462ce1042..4229150668 100644 > --- a/libavfilter/vf_nnedi.c > +++ b/libavfilter/vf_nnedi.c > @@ -637,7 +637,7 @@ static int filter_slice(AVFilterContext *ctx, void > *arg, int jobnr, int nb_jobs) > width, 1, in_scale); > > for (int y = 0; y < slice_end - slice_start; y += 2) { > - if (s->prescreen > 0) > + if (s->pscrn > 0) > s->prescreen[s->pscrn > 1](ctx, srcbuf + (y / 2) * > srcbuf_stride + 32, > srcbuf_stride, prescreen_buf, width, > &s->prescreener[s->pscrn - 1]); > @@ -649,7 +649,7 @@ static int filter_slice(AVFilterContext *ctx, void > *arg, int jobnr, int nb_jobs) > prescreen_buf, width, > &s->coeffs[s->etype][s->nnsparam][s->nsize], > s->qual == 2); > > - if (s->prescreen > 0) > + if (s->pscrn > 0) > interpolation(srcbuf + (y / 2) * srcbuf_stride + 32, > srcbuf_stride, > dstbuf + (y / 2) * dstbuf_stride, > -- > 2.25.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavfilter/vf_nnedi.c b/libavfilter/vf_nnedi.c index 1462ce1042..4229150668 100644 --- a/libavfilter/vf_nnedi.c +++ b/libavfilter/vf_nnedi.c @@ -637,7 +637,7 @@ static int filter_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) width, 1, in_scale); for (int y = 0; y < slice_end - slice_start; y += 2) { - if (s->prescreen > 0) + if (s->pscrn > 0) s->prescreen[s->pscrn > 1](ctx, srcbuf + (y / 2) * srcbuf_stride + 32, srcbuf_stride, prescreen_buf, width, &s->prescreener[s->pscrn - 1]); @@ -649,7 +649,7 @@ static int filter_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) prescreen_buf, width, &s->coeffs[s->etype][s->nnsparam][s->nsize], s->qual == 2); - if (s->prescreen > 0) + if (s->pscrn > 0) interpolation(srcbuf + (y / 2) * srcbuf_stride + 32, srcbuf_stride, dstbuf + (y / 2) * dstbuf_stride,
Since c737f6edcef74a64f4d0ebcefa970bd31266d512 prescreening is nevertheless run because of a wrong check: "if (s->prescreen > 0)". s->prescreen is an array of two function pointers that is contained in the context and comparing it with 0 (i.e. NULL) is actually undefined behaviour, because NULL and s->prescreen do not point to the same object (NULL after all never points to any object). Nevertheless both Clang as well as GCC compile this to code that treat s->prescreen > 0 as true, leading to segfaults, because the code then tries to access the -1th member of an array. This commit fixes the check as well as another such check a few lines below. (Found via compiler warnings enabled by -pedantic: "ordered comparison between pointer and zero is an extension".) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavfilter/vf_nnedi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)