| Message ID | 20210214210923.12099-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/utils: fix undefined behaviour | 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 |
On 2/14/2021 6:09 PM, Paul B Mahol wrote: > Fixes following report: > libavformat/utils.c:1429:14: runtime error: applying zero offset to null pointer How is data NULL here? That's the input packet's data pointer, and this loop is accessed only if size is > 0. data == NULL and size != 0 doesn't sound valid. Or am i missing something? Try compiling with assert level set to 1, see if you get an assertion failure on avpacket helpers. > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavformat/utils.c:1429:14 > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavformat/utils.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 3e955b85bc..e4f100fda2 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, > pkt->pts = pkt->dts = AV_NOPTS_VALUE; > pkt->pos = -1; > /* increment read pointer */ > - data += len; > - size -= len; > + if (len > 0) { > + data += len; > + size -= len; > + } > > got_output = !!out_pkt.size; > >
James Almer: > On 2/14/2021 6:09 PM, Paul B Mahol wrote: >> Fixes following report: >> libavformat/utils.c:1429:14: runtime error: applying zero offset to >> null pointer > > How is data NULL here? That's the input packet's data pointer, and this > loop is accessed only if size is > 0. data == NULL and size != 0 doesn't > sound valid. Or am i missing something? Flushing. > > Try compiling with assert level set to 1, see if you get an assertion > failure on avpacket helpers. > >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior >> libavformat/utils.c:1429:14 >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavformat/utils.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index 3e955b85bc..e4f100fda2 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, >> AVPacket *pkt, >> pkt->pts = pkt->dts = AV_NOPTS_VALUE; >> pkt->pos = -1; >> /* increment read pointer */ >> - data += len; >> - size -= len; >> + if (len > 0) { >> + data += len; >> + size -= len; >> + } >> got_output = !!out_pkt.size; >> > > _______________________________________________ > 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".
On 2/14/2021 6:23 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/14/2021 6:09 PM, Paul B Mahol wrote: >>> Fixes following report: >>> libavformat/utils.c:1429:14: runtime error: applying zero offset to >>> null pointer >> >> How is data NULL here? That's the input packet's data pointer, and this >> loop is accessed only if size is > 0. data == NULL and size != 0 doesn't >> sound valid. Or am i missing something? > > Flushing. A flush packet with data == NULL and size != 0? ff_read_packet(), called before the flush attempt, initializes the packet to defaults. So if it returns < 1, shouldn't the packet remain clean? > >> >> Try compiling with assert level set to 1, see if you get an assertion >> failure on avpacket helpers. >> >>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior >>> libavformat/utils.c:1429:14 >>> >>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>> --- >>> libavformat/utils.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/utils.c b/libavformat/utils.c >>> index 3e955b85bc..e4f100fda2 100644 >>> --- a/libavformat/utils.c >>> +++ b/libavformat/utils.c >>> @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, >>> AVPacket *pkt, >>> pkt->pts = pkt->dts = AV_NOPTS_VALUE; >>> pkt->pos = -1; >>> /* increment read pointer */ >>> - data += len; >>> - size -= len; >>> + if (len > 0) { >>> + data += len; >>> + size -= len; >>> + } >>> got_output = !!out_pkt.size; >>> >> >> _______________________________________________ >> 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". > > _______________________________________________ > 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". >
James Almer: > On 2/14/2021 6:23 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/14/2021 6:09 PM, Paul B Mahol wrote: >>>> Fixes following report: >>>> libavformat/utils.c:1429:14: runtime error: applying zero offset to >>>> null pointer >>> >>> How is data NULL here? That's the input packet's data pointer, and this >>> loop is accessed only if size is > 0. data == NULL and size != 0 doesn't >>> sound valid. Or am i missing something? >> >> Flushing. > > A flush packet with data == NULL and size != 0? ff_read_packet(), called > before the flush attempt, initializes the packet to defaults. So if it > returns < 1, shouldn't the packet remain clean? A flush packet with data == NULL and size == 0. And the parser of course returns 0 in this case, which leads to NULL + 0, which is UB. > >> >>> >>> Try compiling with assert level set to 1, see if you get an assertion >>> failure on avpacket helpers. >>> >>>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior >>>> libavformat/utils.c:1429:14 >>>> >>>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>>> --- >>>> libavformat/utils.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavformat/utils.c b/libavformat/utils.c >>>> index 3e955b85bc..e4f100fda2 100644 >>>> --- a/libavformat/utils.c >>>> +++ b/libavformat/utils.c >>>> @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, >>>> AVPacket *pkt, >>>> pkt->pts = pkt->dts = AV_NOPTS_VALUE; >>>> pkt->pos = -1; >>>> /* increment read pointer */ >>>> - data += len; >>>> - size -= len; >>>> + if (len > 0) { >>>> + data += len; >>>> + size -= len; >>>> + } >>>> got_output = !!out_pkt.size; >>>> >>> >>> _______________________________________________ >>> 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". >> >> _______________________________________________ >> 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". >> > > _______________________________________________ > 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".
On 2/14/2021 6:36 PM, Andreas Rheinhardt wrote: > James Almer: >> On 2/14/2021 6:23 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 2/14/2021 6:09 PM, Paul B Mahol wrote: >>>>> Fixes following report: >>>>> libavformat/utils.c:1429:14: runtime error: applying zero offset to >>>>> null pointer >>>> >>>> How is data NULL here? That's the input packet's data pointer, and this >>>> loop is accessed only if size is > 0. data == NULL and size != 0 doesn't >>>> sound valid. Or am i missing something? >>> >>> Flushing. >> >> A flush packet with data == NULL and size != 0? ff_read_packet(), called >> before the flush attempt, initializes the packet to defaults. So if it >> returns < 1, shouldn't the packet remain clean? > > A flush packet with data == NULL and size == 0. And the parser of course > returns 0 in this case, which leads to NULL + 0, which is UB. Oh, i missed the flush && got_output check that bypasses the size one. Nevermind then. > >> >>> >>>> >>>> Try compiling with assert level set to 1, see if you get an assertion >>>> failure on avpacket helpers. >>>> >>>>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior >>>>> libavformat/utils.c:1429:14 >>>>> >>>>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>>>> --- >>>>> libavformat/utils.c | 6 ++++-- >>>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/libavformat/utils.c b/libavformat/utils.c >>>>> index 3e955b85bc..e4f100fda2 100644 >>>>> --- a/libavformat/utils.c >>>>> +++ b/libavformat/utils.c >>>>> @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, >>>>> AVPacket *pkt, >>>>> pkt->pts = pkt->dts = AV_NOPTS_VALUE; >>>>> pkt->pos = -1; >>>>> /* increment read pointer */ >>>>> - data += len; >>>>> - size -= len; >>>>> + if (len > 0) { >>>>> + data += len; >>>>> + size -= len; >>>>> + } >>>>> got_output = !!out_pkt.size; >>>>> >>>> >>>> _______________________________________________ >>>> 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". >>> >>> _______________________________________________ >>> 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". >>> >> >> _______________________________________________ >> 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". > > _______________________________________________ > 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/libavformat/utils.c b/libavformat/utils.c index 3e955b85bc..e4f100fda2 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -1426,8 +1426,10 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, pkt->pts = pkt->dts = AV_NOPTS_VALUE; pkt->pos = -1; /* increment read pointer */ - data += len; - size -= len; + if (len > 0) { + data += len; + size -= len; + } got_output = !!out_pkt.size;
Fixes following report: libavformat/utils.c:1429:14: runtime error: applying zero offset to null pointer SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavformat/utils.c:1429:14 Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavformat/utils.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)