| Message ID | GV1P250MB0737660624AF52A8561E658B8F549@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avcodec/g723_1enc: Remove unnecessary av_clipl_int32() | expand |
On 9/28/2022 3:58 PM, Andreas Rheinhardt wrote: > This might happen in avio_write() if size == 0 > when the direct codepath is taken. It is undefined behaviour > according to the spec although it happens to work in practice. > Fixes the webm-webvtt-remux FATE-test under UBSan. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/aviobuf.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index b20b1a611a..5b6a42d7f4 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count) > > void avio_write(AVIOContext *s, const unsigned char *buf, int size) > { > + if (size <= 0) > + return; > if (s->direct && !s->update_checksum) { > avio_flush(s); > writeout(s, buf, size); > @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size) > > buf += len; > size -= len; > - } > + } while (size > 0); Why are you adding this at the end of a similar while statement? Did you mean to replace the previous one with do()? > } > > void avio_flush(AVIOContext *s)
James Almer: > On 9/28/2022 3:58 PM, Andreas Rheinhardt wrote: >> This might happen in avio_write() if size == 0 >> when the direct codepath is taken. It is undefined behaviour >> according to the spec although it happens to work in practice. >> Fixes the webm-webvtt-remux FATE-test under UBSan. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/aviobuf.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >> index b20b1a611a..5b6a42d7f4 100644 >> --- a/libavformat/aviobuf.c >> +++ b/libavformat/aviobuf.c >> @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count) >> void avio_write(AVIOContext *s, const unsigned char *buf, int size) >> { >> + if (size <= 0) >> + return; >> if (s->direct && !s->update_checksum) { >> avio_flush(s); >> writeout(s, buf, size); >> @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned >> char *buf, int size) >> buf += len; >> size -= len; >> - } >> + } while (size > 0); > > Why are you adding this at the end of a similar while statement? Did you > mean to replace the previous one with do()? > Yes, exactly. Somehow I didn't. fate passed locally (apart from the tdsc). Crazy that I forgot this. Thanks for spotting. - Andreas
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index b20b1a611a..5b6a42d7f4 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count) void avio_write(AVIOContext *s, const unsigned char *buf, int size) { + if (size <= 0) + return; if (s->direct && !s->update_checksum) { avio_flush(s); writeout(s, buf, size); @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size) buf += len; size -= len; - } + } while (size > 0); } void avio_flush(AVIOContext *s)
This might happen in avio_write() if size == 0 when the direct codepath is taken. It is undefined behaviour according to the spec although it happens to work in practice. Fixes the webm-webvtt-remux FATE-test under UBSan. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/aviobuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)