| Message ID | 20210216155308.3327069-1-jleconte@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavfilter: avoid UB nullptr-with-offset. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | warning | Make fate failed |
Jeremy Leconte: > --- > libavfilter/vf_scale.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > index 58eee96744..98cef5eb4b 100644 > --- a/libavfilter/vf_scale.c > +++ b/libavfilter/vf_scale.c > @@ -631,7 +631,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s > int in_stride[4],out_stride[4]; > int i; > > - for (i=0; i<4; i++) { > + for (i=0; i<4 && cur_pic->data[i] != NULL; i++) { > int vsub= ((i+1)&2) ? scale->vsub : 0; > in_stride[i] = cur_pic->linesize[i] * mul; > out_stride[i] = out_buf->linesize[i] * mul; > Is this just a case of NULL + 0 or is the offset != 0? - Andreas
You're right, it's just a case of NULL + 0. The problem is that it gets caught by clang -fsanitize=undefined. On Tue, Feb 16, 2021 at 4:59 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Jeremy Leconte: > > --- > > libavfilter/vf_scale.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > index 58eee96744..98cef5eb4b 100644 > > --- a/libavfilter/vf_scale.c > > +++ b/libavfilter/vf_scale.c > > @@ -631,7 +631,7 @@ static int scale_slice(AVFilterLink *link, AVFrame > *out_buf, AVFrame *cur_pic, s > > int in_stride[4],out_stride[4]; > > int i; > > > > - for (i=0; i<4; i++) { > > + for (i=0; i<4 && cur_pic->data[i] != NULL; i++) { > > int vsub= ((i+1)&2) ? scale->vsub : 0; > > in_stride[i] = cur_pic->linesize[i] * mul; > > out_stride[i] = out_buf->linesize[i] * mul; > > > Is this just a case of NULL + 0 or is the offset != 0? > > - Andreas > _______________________________________________ > 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".
Andreas Rheinhardt: > Is this just a case of NULL + 0 or is the offset != 0? > > - Andreas > You're right, it's just a case of NULL + 0. My issue is that it gets caught by clang -fsanitize=undefined. Jeremy
On Tue, Feb 16, 2021 at 03:53:08PM +0000, Jeremy Leconte wrote: > --- > libavfilter/vf_scale.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > index 58eee96744..98cef5eb4b 100644 > --- a/libavfilter/vf_scale.c > +++ b/libavfilter/vf_scale.c > @@ -631,7 +631,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s > int in_stride[4],out_stride[4]; > int i; > > - for (i=0; i<4; i++) { > + for (i=0; i<4 && cur_pic->data[i] != NULL; i++) { > int vsub= ((i+1)&2) ? scale->vsub : 0; > in_stride[i] = cur_pic->linesize[i] * mul; > out_stride[i] = out_buf->linesize[i] * mul; This leaves parts of the array uninitialized Ill fix this (unless someone else does before me) thx [...]
Michael Niedermayer: > On Tue, Feb 16, 2021 at 03:53:08PM +0000, Jeremy Leconte wrote: >> --- >> libavfilter/vf_scale.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c >> index 58eee96744..98cef5eb4b 100644 >> --- a/libavfilter/vf_scale.c >> +++ b/libavfilter/vf_scale.c >> @@ -631,7 +631,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s >> int in_stride[4],out_stride[4]; >> int i; >> >> - for (i=0; i<4; i++) { >> + for (i=0; i<4 && cur_pic->data[i] != NULL; i++) { >> int vsub= ((i+1)&2) ? scale->vsub : 0; >> in_stride[i] = cur_pic->linesize[i] * mul; >> out_stride[i] = out_buf->linesize[i] * mul; > > This leaves parts of the array uninitialized > Ill fix this (unless someone else does before me) > I noticed that GCC is smart enough to optimize a ptr = addend ? ptr + addend : addend to a simple ptr += addend (Clang isn't, but future version might do so). Maybe we should put the above in a macro and use it everywhere where such problems exist? (It would of course not fix the case of NULL + addend with addend != 0, but such cases are genuine logic bugs that should not be hidden, but fixed.) - Andreas
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 58eee96744..98cef5eb4b 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -631,7 +631,7 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s int in_stride[4],out_stride[4]; int i; - for (i=0; i<4; i++) { + for (i=0; i<4 && cur_pic->data[i] != NULL; i++) { int vsub= ((i+1)&2) ? scale->vsub : 0; in_stride[i] = cur_pic->linesize[i] * mul; out_stride[i] = out_buf->linesize[i] * mul;