Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
       [not found] <20230906221928.9C116410B55@natalya.videolan.org>
@ 2023-10-21  0:12 ` Michael Niedermayer
  2023-10-21  9:00   ` Paul B Mahol
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2023-10-21  0:12 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 8182 bytes --]

On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote:
> ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Mon Aug 28 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] | committer: Paul B Mahol
> 
> avcodec/magicyuv: add vlc multi support
> 
> Gives nice speed boost, depending on encoded content it goes from
> 30% to 60% faster.
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0
> ---
> 
>  libavcodec/magicyuv.c | 65 +++++++++++++++++++++++++++------------------------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 7898cd5be4..bbaf14d0e0 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -34,6 +34,8 @@
>  #include "lossless_videodsp.h"
>  #include "thread.h"
>  
> +#define VLC_BITS 12
> +
>  typedef struct Slice {
>      uint32_t start;
>      uint32_t size;
> @@ -67,13 +69,14 @@ typedef struct MagicYUVContext {
>      Slice            *slices[4];      // slice bitstream positions for each plane
>      unsigned int      slices_size[4]; // slice sizes for each plane
>      VLC               vlc[4];         // VLC for each plane
> +    VLC_MULTI         multi[4];       // Buffer for joint VLC data
>      int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata,
>                               int j, int threadnr);
>      LLVidDSPContext   llviddsp;
>  } MagicYUVContext;
>  
>  static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
> -                      VLC *vlc, int nb_elems, void *logctx)
> +                      VLC *vlc, VLC_MULTI *multi, int nb_elems, void *logctx)
>  {
>      HuffEntry he[4096];
>  
> @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
>          he[--codes_pos[len[i]]] = (HuffEntry){ len[i], i };
>  
>      ff_free_vlc(vlc);
> -    return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12), nb_elems,
> +    ff_free_vlc_multi(multi);
> +    return ff_init_vlc_multi_from_lengths(vlc, multi, FFMIN(he[0].len, VLC_BITS), nb_elems, nb_elems,
>                                      &he[0].len, sizeof(he[0]),
>                                      &he[0].sym, sizeof(he[0]), sizeof(he[0].sym),
>                                      0, 0, logctx);
> @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t *dst, const uint16_t *src1,
>      *left_top = lt;
>  }
>  
> +#define READ_PLANE(dst, plane, b, c) \
> +{ \
> +    x = 0; \
> +    for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left(&gb) > 0;) {\
> +        ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \
> +                            vlc, vlc_bits, 3); \
> +        if (ret > 0) \
> +            x += ret; \
> +        if (ret <= 0) \
> +            return AVERROR_INVALIDDATA; \
> +    } \
> +    for (; x < width && get_bits_left(&gb) > 0; x++) \
> +        dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \
> +    dst += stride; \
> +}
> +
>  static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
>                                 int j, int threadnr)
>  {
> @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
>          int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
>          ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 + interlaced);
>          ptrdiff_t stride = p->linesize[i] / 2;
> +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> +        const VLCElem *const vlc = s->vlc[i].table;
> +        const int vlc_bits = s->vlc[i].bits;
>          int flags, pred;
>          int ret = init_get_bits8(&gb, s->buf + s->slices[i][j].start,
>                                   s->slices[i][j].size);
> @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
>                  dst += stride;
>              }
>          } else {
> -            for (k = 0; k < height; k++) {
> -                for (x = 0; x < width; x++) {
> -                    int pix;
> -                    if (get_bits_left(&gb) <= 0)
> -                        return AVERROR_INVALIDDATA;
> -
> -                    pix = get_vlc2(&gb, s->vlc[i].table, s->vlc[i].bits, 3);
> -                    if (pix < 0)
> -                        return AVERROR_INVALIDDATA;
> -
> -                    dst[x] = pix;
> -                }
> -                dst += stride;
> -            }
> +            for (k = 0; k < height; k++)
> +                READ_PLANE(dst, i, 2, 3)
>          }
>  
>          switch (pred) {
> @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx, void *tdata,
>          ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
>          ptrdiff_t stride = p->linesize[i];
>          const uint8_t *slice = s->buf + s->slices[i][j].start;
> +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> +        const VLCElem *const vlc = s->vlc[i].table;
> +        const int vlc_bits = s->vlc[i].bits;
>          int flags, pred;
>  
>          flags = bytestream_get_byte(&slice);
> @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx, void *tdata,
>              if (ret < 0)
>                  return ret;
>  
> -            for (k = 0; k < height; k++) {
> -                for (x = 0; x < width; x++) {
> -                    int pix;
> -                    if (get_bits_left(&gb) <= 0)
> -                        return AVERROR_INVALIDDATA;
> -
> -                    pix = get_vlc2(&gb, s->vlc[i].table, s->vlc[i].bits, 3);
> -                    if (pix < 0)
> -                        return AVERROR_INVALIDDATA;
> -
> -                    dst[x] = pix;
> -                }
> -                dst += stride;
> -            }
> +            for (k = 0; k < height; k++)
> +                READ_PLANE(dst, i, 1, 5)
>          }
>  
>          switch (pred) {

Who reviewed this ?

This is a straight out of array write
writing 8 bytes while the check assumes its max 5

==16861==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x631000024800 at pc 0x000001552992 bp 0x7ffd8d34bf10 sp 0x7ffd8d34bf08
WRITE of size 8 at 0x631000024800 thread T0
    #0 0x1552991 in bits_read_vlc_multi_be ffmpeg/libavcodec/bitstream_template.h:531:9
    #1 0x15453d0 in magy_decode_slice ffmpeg/libavcodec/magicyuv.c:302:17
    #2 0x13f3738 in avcodec_default_execute2 ffmpeg/libavcodec/avcodec.c:76:17
    #3 0x153fba5 in magy_decode_frame ffmpeg/libavcodec/magicyuv.c:663:5
    #4 0x142d60e in decode_simple_internal ffmpeg/libavcodec/decode.c:430:20
    #5 0x142d60e in decode_simple_receive_frame ffmpeg/libavcodec/decode.c:609
    #6 0x142d60e in decode_receive_frame_internal ffmpeg/libavcodec/decode.c:637
    #7 0x142c42f in avcodec_send_packet ffmpeg/libavcodec/decode.c:734:15
    #8 0x4ce9df in LLVMFuzzerTestOneInput ffmpeg/tools/target_dec_fuzzer.c:579:25
    #9 0x29f5f0d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
    #10 0x29eaae2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
    #11 0x29efce1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
    #12 0x29ea7c0 in main Fuzzer/build/../FuzzerMain.cpp:20:10
    #13 0x7f15a835ec86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #14 0x420339 in _start (ffmpeg/tools/target_dec_magicyuv_fuzzer+0x420339)


the multi vlc code is not documented, teh function writing out of array
is one of the very few functions in bitstream_template.h that is undocumented

why do we in 2023 add undocumented non trivial functions into files where basically
every function prior is documented ?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
  2023-10-21  0:12 ` [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support Michael Niedermayer
@ 2023-10-21  9:00   ` Paul B Mahol
  2023-10-21 22:56     ` Michael Niedermayer
  0 siblings, 1 reply; 6+ messages in thread
From: Paul B Mahol @ 2023-10-21  9:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote:
> > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Mon Aug 28
> 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] |
> committer: Paul B Mahol
> >
> > avcodec/magicyuv: add vlc multi support
> >
> > Gives nice speed boost, depending on encoded content it goes from
> > 30% to 60% faster.
> >
> > >
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0
> > ---
> >
> >  libavcodec/magicyuv.c | 65
> +++++++++++++++++++++++++++------------------------
> >  1 file changed, 34 insertions(+), 31 deletions(-)
> >
> > diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> > index 7898cd5be4..bbaf14d0e0 100644
> > --- a/libavcodec/magicyuv.c
> > +++ b/libavcodec/magicyuv.c
> > @@ -34,6 +34,8 @@
> >  #include "lossless_videodsp.h"
> >  #include "thread.h"
> >
> > +#define VLC_BITS 12
> > +
> >  typedef struct Slice {
> >      uint32_t start;
> >      uint32_t size;
> > @@ -67,13 +69,14 @@ typedef struct MagicYUVContext {
> >      Slice            *slices[4];      // slice bitstream positions for
> each plane
> >      unsigned int      slices_size[4]; // slice sizes for each plane
> >      VLC               vlc[4];         // VLC for each plane
> > +    VLC_MULTI         multi[4];       // Buffer for joint VLC data
> >      int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata,
> >                               int j, int threadnr);
> >      LLVidDSPContext   llviddsp;
> >  } MagicYUVContext;
> >
> >  static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
> > -                      VLC *vlc, int nb_elems, void *logctx)
> > +                      VLC *vlc, VLC_MULTI *multi, int nb_elems, void
> *logctx)
> >  {
> >      HuffEntry he[4096];
> >
> > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16_t
> codes_pos[33],
> >          he[--codes_pos[len[i]]] = (HuffEntry){ len[i], i };
> >
> >      ff_free_vlc(vlc);
> > -    return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12), nb_elems,
> > +    ff_free_vlc_multi(multi);
> > +    return ff_init_vlc_multi_from_lengths(vlc, multi, FFMIN(he[0].len,
> VLC_BITS), nb_elems, nb_elems,
> >                                      &he[0].len, sizeof(he[0]),
> >                                      &he[0].sym, sizeof(he[0]),
> sizeof(he[0].sym),
> >                                      0, 0, logctx);
> > @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t *dst,
> const uint16_t *src1,
> >      *left_top = lt;
> >  }
> >
> > +#define READ_PLANE(dst, plane, b, c) \
> > +{ \
> > +    x = 0; \
> > +    for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left(&gb)
> > 0;) {\
> > +        ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \
> > +                            vlc, vlc_bits, 3); \
> > +        if (ret > 0) \
> > +            x += ret; \
> > +        if (ret <= 0) \
> > +            return AVERROR_INVALIDDATA; \
> > +    } \
> > +    for (; x < width && get_bits_left(&gb) > 0; x++) \
> > +        dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \
> > +    dst += stride; \
> > +}
> > +
> >  static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
> >                                 int j, int threadnr)
> >  {
> > @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext
> *avctx, void *tdata,
> >          int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
> >          ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 + interlaced);
> >          ptrdiff_t stride = p->linesize[i] / 2;
> > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > +        const VLCElem *const vlc = s->vlc[i].table;
> > +        const int vlc_bits = s->vlc[i].bits;
> >          int flags, pred;
> >          int ret = init_get_bits8(&gb, s->buf + s->slices[i][j].start,
> >                                   s->slices[i][j].size);
> > @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext
> *avctx, void *tdata,
> >                  dst += stride;
> >              }
> >          } else {
> > -            for (k = 0; k < height; k++) {
> > -                for (x = 0; x < width; x++) {
> > -                    int pix;
> > -                    if (get_bits_left(&gb) <= 0)
> > -                        return AVERROR_INVALIDDATA;
> > -
> > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> s->vlc[i].bits, 3);
> > -                    if (pix < 0)
> > -                        return AVERROR_INVALIDDATA;
> > -
> > -                    dst[x] = pix;
> > -                }
> > -                dst += stride;
> > -            }
> > +            for (k = 0; k < height; k++)
> > +                READ_PLANE(dst, i, 2, 3)
> >          }
> >
> >          switch (pred) {
> > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx,
> void *tdata,
> >          ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
> >          ptrdiff_t stride = p->linesize[i];
> >          const uint8_t *slice = s->buf + s->slices[i][j].start;
> > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > +        const VLCElem *const vlc = s->vlc[i].table;
> > +        const int vlc_bits = s->vlc[i].bits;
> >          int flags, pred;
> >
> >          flags = bytestream_get_byte(&slice);
> > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx,
> void *tdata,
> >              if (ret < 0)
> >                  return ret;
> >
> > -            for (k = 0; k < height; k++) {
> > -                for (x = 0; x < width; x++) {
> > -                    int pix;
> > -                    if (get_bits_left(&gb) <= 0)
> > -                        return AVERROR_INVALIDDATA;
> > -
> > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> s->vlc[i].bits, 3);
> > -                    if (pix < 0)
> > -                        return AVERROR_INVALIDDATA;
> > -
> > -                    dst[x] = pix;
> > -                }
> > -                dst += stride;
> > -            }
> > +            for (k = 0; k < height; k++)
> > +                READ_PLANE(dst, i, 1, 5)
> >          }
> >
> >          switch (pred) {
>
> Who reviewed this ?
>
> This is a straight out of array write
> writing 8 bytes while the check assumes its max 5
>

Why are you becoming so aggressive?

The function writes at once 8 bytes, even though only 6 values (in 8bit
case) are kept.

Can you share sample?


> ==16861==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x631000024800 at pc 0x000001552992 bp 0x7ffd8d34bf10 sp 0x7ffd8d34bf08
> WRITE of size 8 at 0x631000024800 thread T0
>     #0 0x1552991 in bits_read_vlc_multi_be
> ffmpeg/libavcodec/bitstream_template.h:531:9
>     #1 0x15453d0 in magy_decode_slice ffmpeg/libavcodec/magicyuv.c:302:17
>     #2 0x13f3738 in avcodec_default_execute2
> ffmpeg/libavcodec/avcodec.c:76:17
>     #3 0x153fba5 in magy_decode_frame ffmpeg/libavcodec/magicyuv.c:663:5
>     #4 0x142d60e in decode_simple_internal
> ffmpeg/libavcodec/decode.c:430:20
>     #5 0x142d60e in decode_simple_receive_frame
> ffmpeg/libavcodec/decode.c:609
>     #6 0x142d60e in decode_receive_frame_internal
> ffmpeg/libavcodec/decode.c:637
>     #7 0x142c42f in avcodec_send_packet ffmpeg/libavcodec/decode.c:734:15
>     #8 0x4ce9df in LLVMFuzzerTestOneInput
> ffmpeg/tools/target_dec_fuzzer.c:579:25
>     #9 0x29f5f0d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*,
> unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13
>     #10 0x29eaae2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*,
> unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6
>     #11 0x29efce1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned
> char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9
>     #12 0x29ea7c0 in main Fuzzer/build/../FuzzerMain.cpp:20:10
>     #13 0x7f15a835ec86 in __libc_start_main
> /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
>     #14 0x420339 in _start
> (ffmpeg/tools/target_dec_magicyuv_fuzzer+0x420339)
>
>
> the multi vlc code is not documented, teh function writing out of array
> is one of the very few functions in bitstream_template.h that is
> undocumented
>
> why do we in 2023 add undocumented non trivial functions into files where
> basically
> every function prior is documented ?
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
  2023-10-21  9:00   ` Paul B Mahol
@ 2023-10-21 22:56     ` Michael Niedermayer
  2023-10-22  7:06       ` Michael Niedermayer
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2023-10-21 22:56 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: cc


[-- Attachment #1.1: Type: text/plain, Size: 7910 bytes --]

On Sat, Oct 21, 2023 at 11:00:19AM +0200, Paul B Mahol wrote:
> On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote:
> > > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Mon Aug 28
> > 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] |
> > committer: Paul B Mahol
> > >
> > > avcodec/magicyuv: add vlc multi support
> > >
> > > Gives nice speed boost, depending on encoded content it goes from
> > > 30% to 60% faster.
> > >
> > > >
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0
> > > ---
> > >
> > >  libavcodec/magicyuv.c | 65
> > +++++++++++++++++++++++++++------------------------
> > >  1 file changed, 34 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> > > index 7898cd5be4..bbaf14d0e0 100644
> > > --- a/libavcodec/magicyuv.c
> > > +++ b/libavcodec/magicyuv.c
> > > @@ -34,6 +34,8 @@
> > >  #include "lossless_videodsp.h"
> > >  #include "thread.h"
> > >
> > > +#define VLC_BITS 12
> > > +
> > >  typedef struct Slice {
> > >      uint32_t start;
> > >      uint32_t size;
> > > @@ -67,13 +69,14 @@ typedef struct MagicYUVContext {
> > >      Slice            *slices[4];      // slice bitstream positions for
> > each plane
> > >      unsigned int      slices_size[4]; // slice sizes for each plane
> > >      VLC               vlc[4];         // VLC for each plane
> > > +    VLC_MULTI         multi[4];       // Buffer for joint VLC data
> > >      int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata,
> > >                               int j, int threadnr);
> > >      LLVidDSPContext   llviddsp;
> > >  } MagicYUVContext;
> > >
> > >  static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
> > > -                      VLC *vlc, int nb_elems, void *logctx)
> > > +                      VLC *vlc, VLC_MULTI *multi, int nb_elems, void
> > *logctx)
> > >  {
> > >      HuffEntry he[4096];
> > >
> > > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16_t
> > codes_pos[33],
> > >          he[--codes_pos[len[i]]] = (HuffEntry){ len[i], i };
> > >
> > >      ff_free_vlc(vlc);
> > > -    return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12), nb_elems,
> > > +    ff_free_vlc_multi(multi);
> > > +    return ff_init_vlc_multi_from_lengths(vlc, multi, FFMIN(he[0].len,
> > VLC_BITS), nb_elems, nb_elems,
> > >                                      &he[0].len, sizeof(he[0]),
> > >                                      &he[0].sym, sizeof(he[0]),
> > sizeof(he[0].sym),
> > >                                      0, 0, logctx);
> > > @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t *dst,
> > const uint16_t *src1,
> > >      *left_top = lt;
> > >  }
> > >
> > > +#define READ_PLANE(dst, plane, b, c) \
> > > +{ \
> > > +    x = 0; \
> > > +    for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left(&gb)
> > > 0;) {\
> > > +        ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \
> > > +                            vlc, vlc_bits, 3); \
> > > +        if (ret > 0) \
> > > +            x += ret; \
> > > +        if (ret <= 0) \
> > > +            return AVERROR_INVALIDDATA; \
> > > +    } \
> > > +    for (; x < width && get_bits_left(&gb) > 0; x++) \
> > > +        dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \
> > > +    dst += stride; \
> > > +}
> > > +
> > >  static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
> > >                                 int j, int threadnr)
> > >  {
> > > @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext
> > *avctx, void *tdata,
> > >          int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
> > >          ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 + interlaced);
> > >          ptrdiff_t stride = p->linesize[i] / 2;
> > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > +        const int vlc_bits = s->vlc[i].bits;
> > >          int flags, pred;
> > >          int ret = init_get_bits8(&gb, s->buf + s->slices[i][j].start,
> > >                                   s->slices[i][j].size);
> > > @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext
> > *avctx, void *tdata,
> > >                  dst += stride;
> > >              }
> > >          } else {
> > > -            for (k = 0; k < height; k++) {
> > > -                for (x = 0; x < width; x++) {
> > > -                    int pix;
> > > -                    if (get_bits_left(&gb) <= 0)
> > > -                        return AVERROR_INVALIDDATA;
> > > -
> > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > s->vlc[i].bits, 3);
> > > -                    if (pix < 0)
> > > -                        return AVERROR_INVALIDDATA;
> > > -
> > > -                    dst[x] = pix;
> > > -                }
> > > -                dst += stride;
> > > -            }
> > > +            for (k = 0; k < height; k++)
> > > +                READ_PLANE(dst, i, 2, 3)
> > >          }
> > >
> > >          switch (pred) {
> > > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx,
> > void *tdata,
> > >          ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
> > >          ptrdiff_t stride = p->linesize[i];
> > >          const uint8_t *slice = s->buf + s->slices[i][j].start;
> > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > +        const int vlc_bits = s->vlc[i].bits;
> > >          int flags, pred;
> > >
> > >          flags = bytestream_get_byte(&slice);
> > > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx,
> > void *tdata,
> > >              if (ret < 0)
> > >                  return ret;
> > >
> > > -            for (k = 0; k < height; k++) {
> > > -                for (x = 0; x < width; x++) {
> > > -                    int pix;
> > > -                    if (get_bits_left(&gb) <= 0)
> > > -                        return AVERROR_INVALIDDATA;
> > > -
> > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > s->vlc[i].bits, 3);
> > > -                    if (pix < 0)
> > > -                        return AVERROR_INVALIDDATA;
> > > -
> > > -                    dst[x] = pix;
> > > -                }
> > > -                dst += stride;
> > > -            }
> > > +            for (k = 0; k < height; k++)
> > > +                READ_PLANE(dst, i, 1, 5)
> > >          }
> > >
> > >          switch (pred) {
> >
> > Who reviewed this ?
> >
> > This is a straight out of array write
> > writing 8 bytes while the check assumes its max 5
> >
> 
> Why are you becoming so aggressive?

I objected to this patchset because of copy and pasted (duplicated) code,
you ignored this objection and pushed it anyway. Now its found out that
it contains a out of array write. After a DOS vulnerability was found
previosuly in it.
You expect me to be happy ?


>
> The function writes at once 8 bytes, even though only 6 values (in 8bit
> case) are kept.
> 

> Can you share sample?

I will mail you what i have but the data is not in a container for ffmpeg
it needs the fuzzer to replicate. I think if you just fix the check to match
the amount of data written that would fix this case. Of course neither that
fix nor the sample will do anything if there are other issues.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
  2023-10-21 22:56     ` Michael Niedermayer
@ 2023-10-22  7:06       ` Michael Niedermayer
  2023-10-22  9:21         ` Paul B Mahol
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2023-10-22  7:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: cc


[-- Attachment #1.1: Type: text/plain, Size: 8084 bytes --]

On Sun, Oct 22, 2023 at 12:56:40AM +0200, Michael Niedermayer wrote:
> On Sat, Oct 21, 2023 at 11:00:19AM +0200, Paul B Mahol wrote:
> > On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <michael@niedermayer.cc>
> > wrote:
> > 
> > > On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote:
> > > > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Mon Aug 28
> > > 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] |
> > > committer: Paul B Mahol
> > > >
> > > > avcodec/magicyuv: add vlc multi support
> > > >
> > > > Gives nice speed boost, depending on encoded content it goes from
> > > > 30% to 60% faster.
> > > >
> > > > >
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0
> > > > ---
> > > >
> > > >  libavcodec/magicyuv.c | 65
> > > +++++++++++++++++++++++++++------------------------
> > > >  1 file changed, 34 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> > > > index 7898cd5be4..bbaf14d0e0 100644
> > > > --- a/libavcodec/magicyuv.c
> > > > +++ b/libavcodec/magicyuv.c
> > > > @@ -34,6 +34,8 @@
> > > >  #include "lossless_videodsp.h"
> > > >  #include "thread.h"
> > > >
> > > > +#define VLC_BITS 12
> > > > +
> > > >  typedef struct Slice {
> > > >      uint32_t start;
> > > >      uint32_t size;
> > > > @@ -67,13 +69,14 @@ typedef struct MagicYUVContext {
> > > >      Slice            *slices[4];      // slice bitstream positions for
> > > each plane
> > > >      unsigned int      slices_size[4]; // slice sizes for each plane
> > > >      VLC               vlc[4];         // VLC for each plane
> > > > +    VLC_MULTI         multi[4];       // Buffer for joint VLC data
> > > >      int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata,
> > > >                               int j, int threadnr);
> > > >      LLVidDSPContext   llviddsp;
> > > >  } MagicYUVContext;
> > > >
> > > >  static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
> > > > -                      VLC *vlc, int nb_elems, void *logctx)
> > > > +                      VLC *vlc, VLC_MULTI *multi, int nb_elems, void
> > > *logctx)
> > > >  {
> > > >      HuffEntry he[4096];
> > > >
> > > > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[], uint16_t
> > > codes_pos[33],
> > > >          he[--codes_pos[len[i]]] = (HuffEntry){ len[i], i };
> > > >
> > > >      ff_free_vlc(vlc);
> > > > -    return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12), nb_elems,
> > > > +    ff_free_vlc_multi(multi);
> > > > +    return ff_init_vlc_multi_from_lengths(vlc, multi, FFMIN(he[0].len,
> > > VLC_BITS), nb_elems, nb_elems,
> > > >                                      &he[0].len, sizeof(he[0]),
> > > >                                      &he[0].sym, sizeof(he[0]),
> > > sizeof(he[0].sym),
> > > >                                      0, 0, logctx);
> > > > @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t *dst,
> > > const uint16_t *src1,
> > > >      *left_top = lt;
> > > >  }
> > > >
> > > > +#define READ_PLANE(dst, plane, b, c) \
> > > > +{ \
> > > > +    x = 0; \
> > > > +    for (; CACHED_BITSTREAM_READER && x < width-c && get_bits_left(&gb)
> > > > 0;) {\
> > > > +        ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \
> > > > +                            vlc, vlc_bits, 3); \
> > > > +        if (ret > 0) \
> > > > +            x += ret; \
> > > > +        if (ret <= 0) \
> > > > +            return AVERROR_INVALIDDATA; \
> > > > +    } \
> > > > +    for (; x < width && get_bits_left(&gb) > 0; x++) \
> > > > +        dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \
> > > > +    dst += stride; \
> > > > +}
> > > > +
> > > >  static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
> > > >                                 int j, int threadnr)
> > > >  {
> > > > @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext
> > > *avctx, void *tdata,
> > > >          int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
> > > >          ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 + interlaced);
> > > >          ptrdiff_t stride = p->linesize[i] / 2;
> > > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > > +        const int vlc_bits = s->vlc[i].bits;
> > > >          int flags, pred;
> > > >          int ret = init_get_bits8(&gb, s->buf + s->slices[i][j].start,
> > > >                                   s->slices[i][j].size);
> > > > @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext
> > > *avctx, void *tdata,
> > > >                  dst += stride;
> > > >              }
> > > >          } else {
> > > > -            for (k = 0; k < height; k++) {
> > > > -                for (x = 0; x < width; x++) {
> > > > -                    int pix;
> > > > -                    if (get_bits_left(&gb) <= 0)
> > > > -                        return AVERROR_INVALIDDATA;
> > > > -
> > > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > > s->vlc[i].bits, 3);
> > > > -                    if (pix < 0)
> > > > -                        return AVERROR_INVALIDDATA;
> > > > -
> > > > -                    dst[x] = pix;
> > > > -                }
> > > > -                dst += stride;
> > > > -            }
> > > > +            for (k = 0; k < height; k++)
> > > > +                READ_PLANE(dst, i, 2, 3)
> > > >          }
> > > >
> > > >          switch (pred) {
> > > > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext *avctx,
> > > void *tdata,
> > > >          ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
> > > >          ptrdiff_t stride = p->linesize[i];
> > > >          const uint8_t *slice = s->buf + s->slices[i][j].start;
> > > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > > +        const int vlc_bits = s->vlc[i].bits;
> > > >          int flags, pred;
> > > >
> > > >          flags = bytestream_get_byte(&slice);
> > > > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext *avctx,
> > > void *tdata,
> > > >              if (ret < 0)
> > > >                  return ret;
> > > >
> > > > -            for (k = 0; k < height; k++) {
> > > > -                for (x = 0; x < width; x++) {
> > > > -                    int pix;
> > > > -                    if (get_bits_left(&gb) <= 0)
> > > > -                        return AVERROR_INVALIDDATA;
> > > > -
> > > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > > s->vlc[i].bits, 3);
> > > > -                    if (pix < 0)
> > > > -                        return AVERROR_INVALIDDATA;
> > > > -
> > > > -                    dst[x] = pix;
> > > > -                }
> > > > -                dst += stride;
> > > > -            }
> > > > +            for (k = 0; k < height; k++)
> > > > +                READ_PLANE(dst, i, 1, 5)
> > > >          }
> > > >
> > > >          switch (pred) {
> > >
> > > Who reviewed this ?
> > >
> > > This is a straight out of array write
> > > writing 8 bytes while the check assumes its max 5
> > >
> > 
> > Why are you becoming so aggressive?
> 
> I objected to this patchset because of copy and pasted (duplicated) code,

as it was disputed, heres the link:
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-September/314414.html


> you ignored this objection and pushed it anyway. Now its found out that
> it contains a out of array write. After a DOS vulnerability was found
> previosuly in it.

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
  2023-10-22  7:06       ` Michael Niedermayer
@ 2023-10-22  9:21         ` Paul B Mahol
  2023-10-22 15:38           ` Michael Niedermayer
  0 siblings, 1 reply; 6+ messages in thread
From: Paul B Mahol @ 2023-10-22  9:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: cc

On Sun, Oct 22, 2023 at 9:06 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sun, Oct 22, 2023 at 12:56:40AM +0200, Michael Niedermayer wrote:
> > On Sat, Oct 21, 2023 at 11:00:19AM +0200, Paul B Mahol wrote:
> > > On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <
> michael@niedermayer.cc>
> > > wrote:
> > >


You have enough skills to fix this,

Good luck!



>
> > > > On Wed, Sep 06, 2023 at 10:19:27PM +0000, Paul B Mahol wrote:
> > > > > ffmpeg | branch: master | Paul B Mahol <onemda@gmail.com> | Mon
> Aug 28
> > > > 12:20:15 2023 +0200| [8b7391cb5ff94ce94612fda69392a95d7ab1ffd0] |
> > > > committer: Paul B Mahol
> > > > >
> > > > > avcodec/magicyuv: add vlc multi support
> > > > >
> > > > > Gives nice speed boost, depending on encoded content it goes from
> > > > > 30% to 60% faster.
> > > > >
> > > > > >
> > > >
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b7391cb5ff94ce94612fda69392a95d7ab1ffd0
> > > > > ---
> > > > >
> > > > >  libavcodec/magicyuv.c | 65
> > > > +++++++++++++++++++++++++++------------------------
> > > > >  1 file changed, 34 insertions(+), 31 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> > > > > index 7898cd5be4..bbaf14d0e0 100644
> > > > > --- a/libavcodec/magicyuv.c
> > > > > +++ b/libavcodec/magicyuv.c
> > > > > @@ -34,6 +34,8 @@
> > > > >  #include "lossless_videodsp.h"
> > > > >  #include "thread.h"
> > > > >
> > > > > +#define VLC_BITS 12
> > > > > +
> > > > >  typedef struct Slice {
> > > > >      uint32_t start;
> > > > >      uint32_t size;
> > > > > @@ -67,13 +69,14 @@ typedef struct MagicYUVContext {
> > > > >      Slice            *slices[4];      // slice bitstream
> positions for
> > > > each plane
> > > > >      unsigned int      slices_size[4]; // slice sizes for each
> plane
> > > > >      VLC               vlc[4];         // VLC for each plane
> > > > > +    VLC_MULTI         multi[4];       // Buffer for joint VLC data
> > > > >      int (*magy_decode_slice)(AVCodecContext *avctx, void *tdata,
> > > > >                               int j, int threadnr);
> > > > >      LLVidDSPContext   llviddsp;
> > > > >  } MagicYUVContext;
> > > > >
> > > > >  static int huff_build(const uint8_t len[], uint16_t codes_pos[33],
> > > > > -                      VLC *vlc, int nb_elems, void *logctx)
> > > > > +                      VLC *vlc, VLC_MULTI *multi, int nb_elems,
> void
> > > > *logctx)
> > > > >  {
> > > > >      HuffEntry he[4096];
> > > > >
> > > > > @@ -84,7 +87,8 @@ static int huff_build(const uint8_t len[],
> uint16_t
> > > > codes_pos[33],
> > > > >          he[--codes_pos[len[i]]] = (HuffEntry){ len[i], i };
> > > > >
> > > > >      ff_free_vlc(vlc);
> > > > > -    return ff_init_vlc_from_lengths(vlc, FFMIN(he[0].len, 12),
> nb_elems,
> > > > > +    ff_free_vlc_multi(multi);
> > > > > +    return ff_init_vlc_multi_from_lengths(vlc, multi,
> FFMIN(he[0].len,
> > > > VLC_BITS), nb_elems, nb_elems,
> > > > >                                      &he[0].len, sizeof(he[0]),
> > > > >                                      &he[0].sym, sizeof(he[0]),
> > > > sizeof(he[0].sym),
> > > > >                                      0, 0, logctx);
> > > > > @@ -111,6 +115,22 @@ static void magicyuv_median_pred16(uint16_t
> *dst,
> > > > const uint16_t *src1,
> > > > >      *left_top = lt;
> > > > >  }
> > > > >
> > > > > +#define READ_PLANE(dst, plane, b, c) \
> > > > > +{ \
> > > > > +    x = 0; \
> > > > > +    for (; CACHED_BITSTREAM_READER && x < width-c &&
> get_bits_left(&gb)
> > > > > 0;) {\
> > > > > +        ret = get_vlc_multi(&gb, (uint8_t *)dst + x * b, multi, \
> > > > > +                            vlc, vlc_bits, 3); \
> > > > > +        if (ret > 0) \
> > > > > +            x += ret; \
> > > > > +        if (ret <= 0) \
> > > > > +            return AVERROR_INVALIDDATA; \
> > > > > +    } \
> > > > > +    for (; x < width && get_bits_left(&gb) > 0; x++) \
> > > > > +        dst[x] = get_vlc2(&gb, vlc, vlc_bits, 3); \
> > > > > +    dst += stride; \
> > > > > +}
> > > > > +
> > > > >  static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
> > > > >                                 int j, int threadnr)
> > > > >  {
> > > > > @@ -130,6 +150,9 @@ static int magy_decode_slice10(AVCodecContext
> > > > *avctx, void *tdata,
> > > > >          int sheight = AV_CEIL_RSHIFT(s->slice_height,
> s->vshift[i]);
> > > > >          ptrdiff_t fake_stride = (p->linesize[i] / 2) * (1 +
> interlaced);
> > > > >          ptrdiff_t stride = p->linesize[i] / 2;
> > > > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > > > +        const int vlc_bits = s->vlc[i].bits;
> > > > >          int flags, pred;
> > > > >          int ret = init_get_bits8(&gb, s->buf +
> s->slices[i][j].start,
> > > > >                                   s->slices[i][j].size);
> > > > > @@ -151,20 +174,8 @@ static int magy_decode_slice10(AVCodecContext
> > > > *avctx, void *tdata,
> > > > >                  dst += stride;
> > > > >              }
> > > > >          } else {
> > > > > -            for (k = 0; k < height; k++) {
> > > > > -                for (x = 0; x < width; x++) {
> > > > > -                    int pix;
> > > > > -                    if (get_bits_left(&gb) <= 0)
> > > > > -                        return AVERROR_INVALIDDATA;
> > > > > -
> > > > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > > > s->vlc[i].bits, 3);
> > > > > -                    if (pix < 0)
> > > > > -                        return AVERROR_INVALIDDATA;
> > > > > -
> > > > > -                    dst[x] = pix;
> > > > > -                }
> > > > > -                dst += stride;
> > > > > -            }
> > > > > +            for (k = 0; k < height; k++)
> > > > > +                READ_PLANE(dst, i, 2, 3)
> > > > >          }
> > > > >
> > > > >          switch (pred) {
> > > > > @@ -261,6 +272,9 @@ static int magy_decode_slice(AVCodecContext
> *avctx,
> > > > void *tdata,
> > > > >          ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
> > > > >          ptrdiff_t stride = p->linesize[i];
> > > > >          const uint8_t *slice = s->buf + s->slices[i][j].start;
> > > > > +        const VLC_MULTI_ELEM *const multi = s->multi[i].table;
> > > > > +        const VLCElem *const vlc = s->vlc[i].table;
> > > > > +        const int vlc_bits = s->vlc[i].bits;
> > > > >          int flags, pred;
> > > > >
> > > > >          flags = bytestream_get_byte(&slice);
> > > > > @@ -280,20 +294,8 @@ static int magy_decode_slice(AVCodecContext
> *avctx,
> > > > void *tdata,
> > > > >              if (ret < 0)
> > > > >                  return ret;
> > > > >
> > > > > -            for (k = 0; k < height; k++) {
> > > > > -                for (x = 0; x < width; x++) {
> > > > > -                    int pix;
> > > > > -                    if (get_bits_left(&gb) <= 0)
> > > > > -                        return AVERROR_INVALIDDATA;
> > > > > -
> > > > > -                    pix = get_vlc2(&gb, s->vlc[i].table,
> > > > s->vlc[i].bits, 3);
> > > > > -                    if (pix < 0)
> > > > > -                        return AVERROR_INVALIDDATA;
> > > > > -
> > > > > -                    dst[x] = pix;
> > > > > -                }
> > > > > -                dst += stride;
> > > > > -            }
> > > > > +            for (k = 0; k < height; k++)
> > > > > +                READ_PLANE(dst, i, 1, 5)
> > > > >          }
> > > > >
> > > > >          switch (pred) {
> > > >
> > > > Who reviewed this ?
> > > >
> > > > This is a straight out of array write
> > > > writing 8 bytes while the check assumes its max 5
> > > >
> > >
> > > Why are you becoming so aggressive?
> >
> > I objected to this patchset because of copy and pasted (duplicated) code,
>
> as it was disputed, heres the link:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-September/314414.html
>
>
> > you ignored this objection and pushed it anyway. Now its found out that
> > it contains a out of array write. After a DOS vulnerability was found
> > previosuly in it.
>
> [...]
>
> thx
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support
  2023-10-22  9:21         ` Paul B Mahol
@ 2023-10-22 15:38           ` Michael Niedermayer
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Niedermayer @ 2023-10-22 15:38 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 806 bytes --]

On Sun, Oct 22, 2023 at 11:21:28AM +0200, Paul B Mahol wrote:
> On Sun, Oct 22, 2023 at 9:06 AM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Sun, Oct 22, 2023 at 12:56:40AM +0200, Michael Niedermayer wrote:
> > > On Sat, Oct 21, 2023 at 11:00:19AM +0200, Paul B Mahol wrote:
> > > > On Sat, Oct 21, 2023 at 2:13 AM Michael Niedermayer <
> > michael@niedermayer.cc>
> > > > wrote:
> > > >
> 
> 
> You have enough skills to fix this,

Of course and i care about the projects security and our users.

will post a fix for this shortly

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-22 15:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20230906221928.9C116410B55@natalya.videolan.org>
2023-10-21  0:12 ` [FFmpeg-devel] [FFmpeg-cvslog] avcodec/magicyuv: add vlc multi support Michael Niedermayer
2023-10-21  9:00   ` Paul B Mahol
2023-10-21 22:56     ` Michael Niedermayer
2023-10-22  7:06       ` Michael Niedermayer
2023-10-22  9:21         ` Paul B Mahol
2023-10-22 15:38           ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git