| Message ID | 20240627004037.1336-1-kasper93@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/jpegxl_parser: ensure input padding is zeroed | 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 |
Kacper Michajłow: > Fixes use of uninitialized value, reported by MSAN. > > Found by OSS-Fuzz. > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > --- > libavcodec/jpegxl_parser.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > index 8c45e1a1b7..f833f844c4 100644 > --- a/libavcodec/jpegxl_parser.c > +++ b/libavcodec/jpegxl_parser.c > @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon > } > cs_buffer = ctx->cs_buffer; > cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied); > + memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE); > } else { > cs_buffer = buf; > cs_buflen = buf_size; This is very strange: The buffer here is part of the parser's private context and this is zeroed initially. So it should never contain uninitialized values. - Andreas
On Thu, 27 Jun 2024 at 02:55, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Kacper Michajłow: > > Fixes use of uninitialized value, reported by MSAN. > > > > Found by OSS-Fuzz. > > > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > > --- > > libavcodec/jpegxl_parser.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > > index 8c45e1a1b7..f833f844c4 100644 > > --- a/libavcodec/jpegxl_parser.c > > +++ b/libavcodec/jpegxl_parser.c > > @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon > > } > > cs_buffer = ctx->cs_buffer; > > cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied); > > + memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE); > > } else { > > cs_buffer = buf; > > cs_buflen = buf_size; > > This is very strange: The buffer here is part of the parser's private > context and this is zeroed initially. So it should never contain > uninitialized values. You are right, but reading this code I had the feeling that it would have the same issue, we read only ctx->copied amount of bytes. We can skip this one if this is certain that this buffer will be clear before reuse always.
On 6/26/24 8:40 PM, Kacper Michajłow wrote: > Fixes use of uninitialized value, reported by MSAN. > > Found by OSS-Fuzz. > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > --- > libavcodec/jpegxl_parser.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c > index 8c45e1a1b7..f833f844c4 100644 > --- a/libavcodec/jpegxl_parser.c > +++ b/libavcodec/jpegxl_parser.c > @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon > } > cs_buffer = ctx->cs_buffer; > cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied); > + memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE); > } else { > cs_buffer = buf; > cs_buflen = buf_size; This one looks fine. Per the other comments on the other jxl patch, I think that one is unnecessary, but this one is fine. I have no comment on the generic patches. - Leo Izen (Traneptora)
diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c index 8c45e1a1b7..f833f844c4 100644 --- a/libavcodec/jpegxl_parser.c +++ b/libavcodec/jpegxl_parser.c @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon } cs_buffer = ctx->cs_buffer; cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied); + memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE); } else { cs_buffer = buf; cs_buflen = buf_size;
Fixes use of uninitialized value, reported by MSAN. Found by OSS-Fuzz. Signed-off-by: Kacper Michajłow <kasper93@gmail.com> --- libavcodec/jpegxl_parser.c | 1 + 1 file changed, 1 insertion(+)