| Message ID | 20200621184208.27665-5-gautamramk@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] libavcodec/jpeg2000.c: Precinct size check removed | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > The log2_chroma_wh is derived from the sample separations of the > codestream if the file is a j2k codestream. Not sure if sample > separation is same is subsampling and whether using sample > separation values from the codestream to determine pixel format. What would get fixed by this change? Carl Eugen
On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > The log2_chroma_wh is derived from the sample separations of the > > codestream if the file is a j2k codestream. Not sure if sample > > separation is same is subsampling and whether using sample > > separation values from the codestream to determine pixel format. > > What would get fixed by this change? > The p1_01.j2k image was not getting recognized by the native decoder due to this condition. It would now get recognized. If this patch is fine, I would preferably remove this check at all places. > Carl Eugen > _______________________________________________ > 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".
Am Mo., 22. Juni 2020 um 04:57 Uhr schrieb Gautam Ramakrishnan <gautamramk@gmail.com>: > > On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > The log2_chroma_wh is derived from the sample separations of the > > > codestream if the file is a j2k codestream. Not sure if sample > > > separation is same is subsampling and whether using sample > > > separation values from the codestream to determine pixel format. > > > > What would get fixed by this change? > > > The p1_01.j2k image was not getting recognized by the native > decoder due to this condition. In any case, this was missing from the commit message. > It would now get recognized. If this patch is fine, I wanted to suggest to add the following two lines after the calls to pix_fmt_guess(): if (s->avctx->pix_fmt == AV_PIX_FMT_NONE && ncomponents == 1) s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; But p1_01.j2k does not get decoded with the change either here. > I would preferably remove this check at all places. I thought the check is needed but if fuzzing does not produce invalid memory access for you, it may be ok. Carl Eugen
On Tue, Jun 23, 2020 at 2:55 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Mo., 22. Juni 2020 um 04:57 Uhr schrieb Gautam Ramakrishnan > <gautamramk@gmail.com>: > > > > On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > > > The log2_chroma_wh is derived from the sample separations of the > > > > codestream if the file is a j2k codestream. Not sure if sample > > > > separation is same is subsampling and whether using sample > > > > separation values from the codestream to determine pixel format. > > > > > > What would get fixed by this change? > > > > > The p1_01.j2k image was not getting recognized by the native > > decoder due to this condition. > > In any case, this was missing from the commit message. > > > It would now get recognized. If this patch is fine, > > I wanted to suggest to add the following two lines after > the calls to pix_fmt_guess(): > if (s->avctx->pix_fmt == AV_PIX_FMT_NONE && ncomponents == 1) > s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > I had tried this for testing initially. I placed this inside the if (i == possible_fmts_nb) check. It seemed to work correctly. I could possibly resend with this also, but I did not know this would be a good solution. > But p1_01.j2k does not get decoded with the change either here. > > > I would preferably remove this check at all places. > > I thought the check is needed but if fuzzing does not produce > invalid memory access for you, it may be ok. > I'll run the fuzzer again carefully. > Carl Eugen > _______________________________________________
On Tue, Jun 23, 2020 at 8:04 AM Gautam Ramakrishnan <gautamramk@gmail.com> wrote: > > On Tue, Jun 23, 2020 at 2:55 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > Am Mo., 22. Juni 2020 um 04:57 Uhr schrieb Gautam Ramakrishnan > > <gautamramk@gmail.com>: > > > > > > On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > > > > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > > > > > The log2_chroma_wh is derived from the sample separations of the > > > > > codestream if the file is a j2k codestream. Not sure if sample > > > > > separation is same is subsampling and whether using sample > > > > > separation values from the codestream to determine pixel format. > > > > > > > > What would get fixed by this change? > > > > > > > The p1_01.j2k image was not getting recognized by the native > > > decoder due to this condition. > > > > In any case, this was missing from the commit message. > > > > > It would now get recognized. If this patch is fine, > > > > I wanted to suggest to add the following two lines after > > the calls to pix_fmt_guess(): > > if (s->avctx->pix_fmt == AV_PIX_FMT_NONE && ncomponents == 1) > > s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > > > I had tried this for testing initially. I placed this inside the > if (i == possible_fmts_nb) check. It seemed to work correctly. > I could possibly resend with this also, but I did not know this would > be a good solution. > > But p1_01.j2k does not get decoded with the change either here. > > > > > I would preferably remove this check at all places. > > > > I thought the check is needed but if fuzzing does not produce > > invalid memory access for you, it may be ok. > > > I'll run the fuzzer again carefully. I ran the fuzzer (zzuf) where I ran ffmpeg with input files p1_01.j2k and p1_07.j2k. I tried with seeds from 0 to 10000. I tried error rates of 0.01, 0.1 and 0.5. There was no segfault. I used the -c option as I only wanted it to fuzz the .j2k files. I hope my configuration while using zzuf was correct. > > Carl Eugen > > _______________________________________________ > > > > > -- > ------------- > Gautam |
Am Do., 25. Juni 2020 um 20:52 Uhr schrieb Gautam Ramakrishnan <gautamramk@gmail.com>: > I ran the fuzzer (zzuf) where I ran ffmpeg with input files p1_01.j2k > and p1_07.j2k. > I tried with seeds from 0 to 10000. > I tried error rates of 0.01, 0.1 and 0.5. There was no segfault. I > used the -c option as > I only wanted it to fuzz the .j2k files. I hope my configuration while > using zzuf was correct. You don't have to show all lines you tested, but one (or two) might help to answer this... Carl Eugen
On Fri, Jun 26, 2020 at 12:22:22AM +0530, Gautam Ramakrishnan wrote: > On Tue, Jun 23, 2020 at 8:04 AM Gautam Ramakrishnan > <gautamramk@gmail.com> wrote: > > > > On Tue, Jun 23, 2020 at 2:55 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > Am Mo., 22. Juni 2020 um 04:57 Uhr schrieb Gautam Ramakrishnan > > > <gautamramk@gmail.com>: > > > > > > > > On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > > > > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > > > > > > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > > > > > > > The log2_chroma_wh is derived from the sample separations of the > > > > > > codestream if the file is a j2k codestream. Not sure if sample > > > > > > separation is same is subsampling and whether using sample > > > > > > separation values from the codestream to determine pixel format. > > > > > > > > > > What would get fixed by this change? > > > > > > > > > The p1_01.j2k image was not getting recognized by the native > > > > decoder due to this condition. > > > > > > In any case, this was missing from the commit message. > > > > > > > It would now get recognized. If this patch is fine, > > > > > > I wanted to suggest to add the following two lines after > > > the calls to pix_fmt_guess(): > > > if (s->avctx->pix_fmt == AV_PIX_FMT_NONE && ncomponents == 1) > > > s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > > > > > I had tried this for testing initially. I placed this inside the > > if (i == possible_fmts_nb) check. It seemed to work correctly. > > I could possibly resend with this also, but I did not know this would > > be a good solution. > > > But p1_01.j2k does not get decoded with the change either here. > > > > > > > I would preferably remove this check at all places. > > > > > > I thought the check is needed but if fuzzing does not produce > > > invalid memory access for you, it may be ok. > > > > > I'll run the fuzzer again carefully. > I ran the fuzzer (zzuf) where I ran ffmpeg with input files p1_01.j2k > and p1_07.j2k. > I tried with seeds from 0 to 10000. > I tried error rates of 0.01, 0.1 and 0.5. There was no segfault. I > used the -c option as > I only wanted it to fuzz the .j2k files. I hope my configuration while > using zzuf was correct. from my command line history, i tested zzuf -C9 -s 0:100 ./ffmpeg -v -99 -i p1_07.j2k zzuf[s=8,r=0.004]: signal 11 (SIGSEGV) zzuf[s=12,r=0.004]: signal 11 (SIGSEGV) zzuf[s=52,r=0.004]: signal 11 (SIGSEGV) zzuf[s=81,r=0.004]: signal 11 (SIGSEGV) zzuf[s=93,r=0.004]: signal 11 (SIGSEGV) zzuf[s=97,r=0.004]: signal 11 (SIGSEGV) i didnt investigate these at all yet so they may be unrelated to the j2k decoder, but there where segfaults ... thx [...]
On Fri, Jun 26, 2020 at 3:14 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Jun 26, 2020 at 12:22:22AM +0530, Gautam Ramakrishnan wrote: > > On Tue, Jun 23, 2020 at 8:04 AM Gautam Ramakrishnan > > <gautamramk@gmail.com> wrote: > > > > > > On Tue, Jun 23, 2020 at 2:55 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > > > Am Mo., 22. Juni 2020 um 04:57 Uhr schrieb Gautam Ramakrishnan > > > > <gautamramk@gmail.com>: > > > > > > > > > > On Mon, Jun 22, 2020 at 1:54 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > > > > > > > Am So., 21. Juni 2020 um 21:11 Uhr schrieb <gautamramk@gmail.com>: > > > > > > > > > > > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > > > > > > > > > > > The log2_chroma_wh is derived from the sample separations of the > > > > > > > codestream if the file is a j2k codestream. Not sure if sample > > > > > > > separation is same is subsampling and whether using sample > > > > > > > separation values from the codestream to determine pixel format. > > > > > > > > > > > > What would get fixed by this change? > > > > > > > > > > > The p1_01.j2k image was not getting recognized by the native > > > > > decoder due to this condition. > > > > > > > > In any case, this was missing from the commit message. > > > > > > > > > It would now get recognized. If this patch is fine, > > > > > > > > I wanted to suggest to add the following two lines after > > > > the calls to pix_fmt_guess(): > > > > if (s->avctx->pix_fmt == AV_PIX_FMT_NONE && ncomponents == 1) > > > > s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > > > > > > > I had tried this for testing initially. I placed this inside the > > > if (i == possible_fmts_nb) check. It seemed to work correctly. > > > I could possibly resend with this also, but I did not know this would > > > be a good solution. > > > > But p1_01.j2k does not get decoded with the change either here. > > > > > > > > > I would preferably remove this check at all places. > > > > > > > > I thought the check is needed but if fuzzing does not produce > > > > invalid memory access for you, it may be ok. > > > > > > > I'll run the fuzzer again carefully. > > I ran the fuzzer (zzuf) where I ran ffmpeg with input files p1_01.j2k > > and p1_07.j2k. > > I tried with seeds from 0 to 10000. > > I tried error rates of 0.01, 0.1 and 0.5. There was no segfault. I > > used the -c option as > > I only wanted it to fuzz the .j2k files. I hope my configuration while > > using zzuf was correct. > > from my command line history, i tested > zzuf -C9 -s 0:100 ./ffmpeg -v -99 -i p1_07.j2k > > zzuf[s=8,r=0.004]: signal 11 (SIGSEGV) > zzuf[s=12,r=0.004]: signal 11 (SIGSEGV) > zzuf[s=52,r=0.004]: signal 11 (SIGSEGV) > zzuf[s=81,r=0.004]: signal 11 (SIGSEGV) > zzuf[s=93,r=0.004]: signal 11 (SIGSEGV) > zzuf[s=97,r=0.004]: signal 11 (SIGSEGV) > > i didnt investigate these at all yet so they may be unrelated to the j2k > decoder, but there where segfaults ... I tried the same command. However, even if I remove the input file from ffmpeg, i.e run zzuf -C9 -s 0:100 ./ffmpeg -v -99, I get segfaults. Is it possible that ffmpeg reads some other files which is getting fuzzed? In a tutorial, I saw that using -c command line option with zzuf will ensure only files which show up on command line will get fuzzed. When I run with the -c argument, I get no segfaults > > thx > -- ------------- Gautam |
On Mon, Jun 22, 2020 at 12:12:08AM +0530, gautamramk@gmail.com wrote: > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > The log2_chroma_wh is derived from the sample separations of the > codestream if the file is a j2k codestream. Not sure if sample > separation is same is subsampling and whether using sample > separation values from the codestream to determine pixel format. > --- > libavcodec/jpeg2000dec.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > index c8c89803ac..2b9659bf96 100644 > --- a/libavcodec/jpeg2000dec.c > +++ b/libavcodec/jpeg2000dec.c > @@ -225,8 +225,6 @@ static int pix_fmt_match(enum AVPixelFormat pix_fmt, int components, > > case 1: > match = match && desc->comp[0].depth >= bpc && > - (log2_chroma_wh >> 2 & 3) == 0 && > - (log2_chroma_wh & 3) == 0 && > (desc->flags & AV_PIX_FMT_FLAG_PAL) == pal8 * AV_PIX_FMT_FLAG_PAL; > } > return match; Heres a better bug report ffmpeg_g -i afl-testcase/p0_10.j2k -f null - (the testcase should be the normal reference file before fuzzing, this crashed before any fuzzing) [...] [jpeg2000 @ 0x555559d70880] bpno became invalid Last message repeated 4 times Program received signal SIGSEGV, Segmentation fault. ff_jpeg2000_cleanup (comp=0x555559d76d40, codsty=0x555559d7f98c) at libavcodec/jpeg2000.c:604 604 int nb_code_blocks = prec->nb_codeblocks_height * prec->nb_codeblocks_width; (gdb) bt #0 0x0000555557043463 in ff_jpeg2000_cleanup (comp=0x555559d76d40, codsty=0x555559d7f98c) at libavcodec/jpeg2000.c:604 #1 0x0000555557045bc5 in jpeg2000_dec_cleanup (s=s@entry=0x555559d71fc0) at libavcodec/jpeg2000dec.c:2029 #2 0x000055555706263c in jpeg2000_decode_frame (avctx=0x555559d70880, data=0x555559d71880, got_frame=0x7fffffffd5e0, avpkt=<optimized out>) at libavcodec/jpeg2000dec.c:2416 #3 0x0000555556aad261 in decode_simple_internal (frame=<optimized out>, avctx=<optimized out>) at libavcodec/decode.c:342 #4 0x0000555556aad261 in decode_simple_receive_frame (frame=<optimized out>, avctx=<optimized out>) at libavcodec/decode.c:538 #5 0x0000555556aad261 in decode_receive_frame_internal (avctx=avctx@entry=0x555559d70880, frame=0x555559d71880) at libavcodec/decode.c:556 #6 0x0000555556aafa28 in avcodec_send_packet (avctx=avctx@entry=0x555559d70880, avpkt=avpkt@entry=0x7fffffffd6b0) at libavcodec/decode.c:614 #7 0x00005555567c9f23 in try_decode_frame (s=s@entry=0x555559d6e940, st=st@entry=0x555559d6fe00, avpkt=avpkt@entry=0x555559d76940, options=<optimized out>) at libavformat/utils.c:3111 #8 0x00005555567fad2a in avformat_find_stream_info (ic=0x555559d6e940, options=0x555559d70740) at libavformat/utils.c:3954 #9 0x00005555558c6eb9 in open_input_file (o=o@entry=0x7fffffffdcb0, filename=<optimized out>) at fftools/ffmpeg_opt.c:1185 #10 0x00005555558d1ea9 in open_files (l=0x555559d6e718, l=0x555559d6e718, open_file=0x5555558c2410 <open_input_file>, inout=0x555558c178b9 "input") at fftools/ffmpeg_opt.c:3302 #11 0x00005555558d1ea9 in ffmpeg_parse_options (argc=<optimized out>, argv=<optimized out>) at fftools/ffmpeg_opt.c:3342 #12 0x00005555558abcb9 in main (argc=6, argv=0x7fffffffe258) at fftools/ffmpeg.c:4848 [...]
On Sat, Jun 27, 2020 at 5:04 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Jun 22, 2020 at 12:12:08AM +0530, gautamramk@gmail.com wrote: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > The log2_chroma_wh is derived from the sample separations of the > > codestream if the file is a j2k codestream. Not sure if sample > > separation is same is subsampling and whether using sample > > separation values from the codestream to determine pixel format. > > --- > > libavcodec/jpeg2000dec.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c > > index c8c89803ac..2b9659bf96 100644 > > --- a/libavcodec/jpeg2000dec.c > > +++ b/libavcodec/jpeg2000dec.c > > @@ -225,8 +225,6 @@ static int pix_fmt_match(enum AVPixelFormat pix_fmt, int components, > > > > case 1: > > match = match && desc->comp[0].depth >= bpc && > > - (log2_chroma_wh >> 2 & 3) == 0 && > > - (log2_chroma_wh & 3) == 0 && > > (desc->flags & AV_PIX_FMT_FLAG_PAL) == pal8 * AV_PIX_FMT_FLAG_PAL; > > } > > return match; > > Heres a better bug report > > ffmpeg_g -i afl-testcase/p0_10.j2k -f null - > (the testcase should be the normal reference file before fuzzing, this crashed > before any fuzzing) > > [...] > [jpeg2000 @ 0x555559d70880] bpno became invalid > Last message repeated 4 times > Program received signal SIGSEGV, Segmentation fault. > ff_jpeg2000_cleanup (comp=0x555559d76d40, codsty=0x555559d7f98c) at libavcodec/jpeg2000.c:604 > 604 int nb_code_blocks = prec->nb_codeblocks_height * prec->nb_codeblocks_width; > (gdb) bt > #0 0x0000555557043463 in ff_jpeg2000_cleanup (comp=0x555559d76d40, codsty=0x555559d7f98c) at libavcodec/jpeg2000.c:604 > #1 0x0000555557045bc5 in jpeg2000_dec_cleanup (s=s@entry=0x555559d71fc0) at libavcodec/jpeg2000dec.c:2029 > #2 0x000055555706263c in jpeg2000_decode_frame (avctx=0x555559d70880, data=0x555559d71880, got_frame=0x7fffffffd5e0, avpkt=<optimized out>) at libavcodec/jpeg2000dec.c:2416 > #3 0x0000555556aad261 in decode_simple_internal (frame=<optimized out>, avctx=<optimized out>) at libavcodec/decode.c:342 > #4 0x0000555556aad261 in decode_simple_receive_frame (frame=<optimized out>, avctx=<optimized out>) at libavcodec/decode.c:538 > #5 0x0000555556aad261 in decode_receive_frame_internal (avctx=avctx@entry=0x555559d70880, frame=0x555559d71880) at libavcodec/decode.c:556 > #6 0x0000555556aafa28 in avcodec_send_packet (avctx=avctx@entry=0x555559d70880, avpkt=avpkt@entry=0x7fffffffd6b0) at libavcodec/decode.c:614 > #7 0x00005555567c9f23 in try_decode_frame (s=s@entry=0x555559d6e940, st=st@entry=0x555559d6fe00, avpkt=avpkt@entry=0x555559d76940, options=<optimized out>) at libavformat/utils.c:3111 > #8 0x00005555567fad2a in avformat_find_stream_info (ic=0x555559d6e940, options=0x555559d70740) at libavformat/utils.c:3954 > #9 0x00005555558c6eb9 in open_input_file (o=o@entry=0x7fffffffdcb0, filename=<optimized out>) at fftools/ffmpeg_opt.c:1185 > #10 0x00005555558d1ea9 in open_files (l=0x555559d6e718, l=0x555559d6e718, open_file=0x5555558c2410 <open_input_file>, inout=0x555558c178b9 "input") at fftools/ffmpeg_opt.c:3302 > #11 0x00005555558d1ea9 in ffmpeg_parse_options (argc=<optimized out>, argv=<optimized out>) at fftools/ffmpeg_opt.c:3342 > #12 0x00005555558abcb9 in main (argc=6, argv=0x7fffffffe258) at fftools/ffmpeg.c:4848 Thanks Michael, I'll look for errors in the code.
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index c8c89803ac..2b9659bf96 100644 --- a/libavcodec/jpeg2000dec.c +++ b/libavcodec/jpeg2000dec.c @@ -225,8 +225,6 @@ static int pix_fmt_match(enum AVPixelFormat pix_fmt, int components, case 1: match = match && desc->comp[0].depth >= bpc && - (log2_chroma_wh >> 2 & 3) == 0 && - (log2_chroma_wh & 3) == 0 && (desc->flags & AV_PIX_FMT_FLAG_PAL) == pal8 * AV_PIX_FMT_FLAG_PAL; } return match;
From: Gautam Ramakrishnan <gautamramk@gmail.com> The log2_chroma_wh is derived from the sample separations of the codestream if the file is a j2k codestream. Not sure if sample separation is same is subsampling and whether using sample separation values from the codestream to determine pixel format. --- libavcodec/jpeg2000dec.c | 2 -- 1 file changed, 2 deletions(-)