* [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice.
@ 2024-04-21 14:52 Nuo Mi
2024-04-21 16:34 ` Frank Plowman
0 siblings, 1 reply; 3+ messages in thread
From: Nuo Mi @ 2024-04-21 14:52 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Frank Plowman, Nuo Mi
For some error bitstreams, a CTU belongs to two slices/entry points.
If the decoder initializes and submmits the CTU task twice, it may crash the program
or cause it to enter an infinite loop.
Reported-by: Frank Plowman <post@frankplowman.com>
---
libavcodec/vvc/dec.c | 7 +++++--
libavcodec/vvc/thread.c | 43 ++++++++++++++++++++++++++++-------------
libavcodec/vvc/thread.h | 2 +-
3 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index 6aeec27eaf..4f7d184e43 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -893,10 +893,13 @@ static int wait_delayed_frame(VVCContext *s, AVFrame *output, int *got_output)
static int submit_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *output, int *got_output)
{
- int ret;
+ int ret = ff_vvc_frame_submit(s, fc);
+ if (ret < 0)
+ return ret;
+
s->nb_frames++;
s->nb_delayed++;
- ff_vvc_frame_submit(s, fc);
+
if (s->nb_delayed >= s->nb_fcs) {
if ((ret = wait_delayed_frame(s, output, got_output)) < 0)
return ret;
diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index 01c3ff75b1..3b27811db2 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -124,11 +124,17 @@ static void task_init(VVCTask *t, VVCTaskStage stage, VVCFrameContext *fc, const
atomic_store(&t->target_inter_score, 0);
}
-static void task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep, const int ctu_idx)
+static int task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep, const int ctu_idx)
{
+ if (t->sc) {
+ // the task already inited, error bitstream
+ return AVERROR_INVALIDDATA;
+ }
t->sc = sc;
t->ep = ep;
t->ctu_idx = ctu_idx;
+
+ return 0;
}
static uint8_t task_add_score(VVCTask *t, const VVCTaskStage stage)
@@ -758,24 +764,35 @@ static void submit_entry_point(VVCContext *s, VVCFrameThread *ft, SliceContext *
frame_thread_add_score(s, ft, t->rx, t->ry, VVC_TASK_STAGE_PARSE);
}
-void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
+int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
{
VVCFrameThread *ft = fc->ft;
- for (int i = 0; i < fc->nb_slices; i++) {
- SliceContext *sc = fc->slices[i];
- for (int j = 0; j < sc->nb_eps; j++) {
- EntryPoint *ep = sc->eps + j;
- for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
- const int rs = sc->sh.ctb_addr_in_curr_slice[k];
- VVCTask *t = ft->tasks + rs;
-
- task_init_parse(t, sc, ep, k);
- check_colocation(s, t);
+ // We'll handle this in two passes:
+ // Pass 0 to initialize tasks with parser, this will help detect bit stream error
+ // Pass 1 to shedule location check and submit the entry point
+ for (int pass = 0; pass < 2; pass++) {
+ for (int i = 0; i < fc->nb_slices; i++) {
+ SliceContext *sc = fc->slices[i];
+ for (int j = 0; j < sc->nb_eps; j++) {
+ EntryPoint *ep = sc->eps + j;
+ for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
+ const int rs = sc->sh.ctb_addr_in_curr_slice[k];
+ VVCTask *t = ft->tasks + rs;
+ if (pass) {
+ check_colocation(s, t);
+ } else {
+ const int ret = task_init_parse(t, sc, ep, k);
+ if (ret < 0)
+ return ret;
+ }
+ }
+ if (pass)
+ submit_entry_point(s, ft, sc, ep);
}
- submit_entry_point(s, ft, sc, ep);
}
}
+ return 0;
}
int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc)
diff --git a/libavcodec/vvc/thread.h b/libavcodec/vvc/thread.h
index 55bb4ea244..8ac59b2ecf 100644
--- a/libavcodec/vvc/thread.h
+++ b/libavcodec/vvc/thread.h
@@ -30,7 +30,7 @@ void ff_vvc_executor_free(struct AVExecutor **e);
int ff_vvc_frame_thread_init(VVCFrameContext *fc);
void ff_vvc_frame_thread_free(VVCFrameContext *fc);
-void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
+int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc);
#endif // AVCODEC_VVC_THREAD_H
--
2.34.1
_______________________________________________
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] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice.
2024-04-21 14:52 [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice Nuo Mi
@ 2024-04-21 16:34 ` Frank Plowman
2024-04-25 14:05 ` Nuo Mi
0 siblings, 1 reply; 3+ messages in thread
From: Frank Plowman @ 2024-04-21 16:34 UTC (permalink / raw)
To: ffmpeg-devel
On 21/04/2024 15:52, Nuo Mi wrote:
> For some error bitstreams, a CTU belongs to two slices/entry points.
> If the decoder initializes and submmits the CTU task twice, it may crash the program
> or cause it to enter an infinite loop.
>
> Reported-by: Frank Plowman <post@frankplowman.com>
> ---
> libavcodec/vvc/dec.c | 7 +++++--
> libavcodec/vvc/thread.c | 43 ++++++++++++++++++++++++++++-------------
> libavcodec/vvc/thread.h | 2 +-
> 3 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
> index 6aeec27eaf..4f7d184e43 100644
> --- a/libavcodec/vvc/dec.c
> +++ b/libavcodec/vvc/dec.c
> @@ -893,10 +893,13 @@ static int wait_delayed_frame(VVCContext *s, AVFrame *output, int *got_output)
>
> static int submit_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *output, int *got_output)
> {
> - int ret;
> + int ret = ff_vvc_frame_submit(s, fc);
> + if (ret < 0)
> + return ret;
> +
> s->nb_frames++;
> s->nb_delayed++;
> - ff_vvc_frame_submit(s, fc);
> +
> if (s->nb_delayed >= s->nb_fcs) {
> if ((ret = wait_delayed_frame(s, output, got_output)) < 0)
> return ret;
> diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
> index 01c3ff75b1..3b27811db2 100644
> --- a/libavcodec/vvc/thread.c
> +++ b/libavcodec/vvc/thread.c
> @@ -124,11 +124,17 @@ static void task_init(VVCTask *t, VVCTaskStage stage, VVCFrameContext *fc, const
> atomic_store(&t->target_inter_score, 0);
> }
>
> -static void task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep, const int ctu_idx)
> +static int task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep, const int ctu_idx)
> {
> + if (t->sc) {
> + // the task already inited, error bitstream
> + return AVERROR_INVALIDDATA;
> + }
> t->sc = sc;
> t->ep = ep;
> t->ctu_idx = ctu_idx;
> +
> + return 0;
> }
>
> static uint8_t task_add_score(VVCTask *t, const VVCTaskStage stage)
> @@ -758,24 +764,35 @@ static void submit_entry_point(VVCContext *s, VVCFrameThread *ft, SliceContext *
> frame_thread_add_score(s, ft, t->rx, t->ry, VVC_TASK_STAGE_PARSE);
> }
>
> -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> +int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> {
> VVCFrameThread *ft = fc->ft;
>
> - for (int i = 0; i < fc->nb_slices; i++) {
> - SliceContext *sc = fc->slices[i];
> - for (int j = 0; j < sc->nb_eps; j++) {
> - EntryPoint *ep = sc->eps + j;
> - for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> - const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> - VVCTask *t = ft->tasks + rs;
> -
> - task_init_parse(t, sc, ep, k);
> - check_colocation(s, t);
> + // We'll handle this in two passes:
> + // Pass 0 to initialize tasks with parser, this will help detect bit stream error
> + // Pass 1 to shedule location check and submit the entry point
> + for (int pass = 0; pass < 2; pass++) {
> + for (int i = 0; i < fc->nb_slices; i++) {
> + SliceContext *sc = fc->slices[i];
> + for (int j = 0; j < sc->nb_eps; j++) {
> + EntryPoint *ep = sc->eps + j;
> + for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> + const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> + VVCTask *t = ft->tasks + rs;
> + if (pass) {
> + check_colocation(s, t);
> + } else {
> + const int ret = task_init_parse(t, sc, ep, k);
> + if (ret < 0)
> + return ret;
> + }
> + }
> + if (pass)
> + submit_entry_point(s, ft, sc, ep);
> }
> - submit_entry_point(s, ft, sc, ep);
> }
> }
> + return 0;
> }
>
> int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc)
> diff --git a/libavcodec/vvc/thread.h b/libavcodec/vvc/thread.h
> index 55bb4ea244..8ac59b2ecf 100644
> --- a/libavcodec/vvc/thread.h
> +++ b/libavcodec/vvc/thread.h
> @@ -30,7 +30,7 @@ void ff_vvc_executor_free(struct AVExecutor **e);
>
> int ff_vvc_frame_thread_init(VVCFrameContext *fc);
> void ff_vvc_frame_thread_free(VVCFrameContext *fc);
> -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
> +int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
> int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc);
>
> #endif // AVCODEC_VVC_THREAD_H
This patch fixes most of the fuzz bitstreams I have which enter infinite
loops, but also introduces a regression, turning some other bitstreams
which were okay before into infinite loops.
--
Frank
_______________________________________________
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] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice.
2024-04-21 16:34 ` Frank Plowman
@ 2024-04-25 14:05 ` Nuo Mi
0 siblings, 0 replies; 3+ messages in thread
From: Nuo Mi @ 2024-04-25 14:05 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Thank you, Frank
Fixed by v2
On Mon, Apr 22, 2024 at 12:34 AM Frank Plowman <post@frankplowman.com>
wrote:
> On 21/04/2024 15:52, Nuo Mi wrote:
> > For some error bitstreams, a CTU belongs to two slices/entry points.
> > If the decoder initializes and submmits the CTU task twice, it may crash
> the program
> > or cause it to enter an infinite loop.
> >
> > Reported-by: Frank Plowman <post@frankplowman.com>
> > ---
> > libavcodec/vvc/dec.c | 7 +++++--
> > libavcodec/vvc/thread.c | 43 ++++++++++++++++++++++++++++-------------
> > libavcodec/vvc/thread.h | 2 +-
> > 3 files changed, 36 insertions(+), 16 deletions(-)
> >
> > diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
> > index 6aeec27eaf..4f7d184e43 100644
> > --- a/libavcodec/vvc/dec.c
> > +++ b/libavcodec/vvc/dec.c
> > @@ -893,10 +893,13 @@ static int wait_delayed_frame(VVCContext *s,
> AVFrame *output, int *got_output)
> >
> > static int submit_frame(VVCContext *s, VVCFrameContext *fc, AVFrame
> *output, int *got_output)
> > {
> > - int ret;
> > + int ret = ff_vvc_frame_submit(s, fc);
> > + if (ret < 0)
> > + return ret;
> > +
> > s->nb_frames++;
> > s->nb_delayed++;
> > - ff_vvc_frame_submit(s, fc);
> > +
> > if (s->nb_delayed >= s->nb_fcs) {
> > if ((ret = wait_delayed_frame(s, output, got_output)) < 0)
> > return ret;
> > diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
> > index 01c3ff75b1..3b27811db2 100644
> > --- a/libavcodec/vvc/thread.c
> > +++ b/libavcodec/vvc/thread.c
> > @@ -124,11 +124,17 @@ static void task_init(VVCTask *t, VVCTaskStage
> stage, VVCFrameContext *fc, const
> > atomic_store(&t->target_inter_score, 0);
> > }
> >
> > -static void task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint
> *ep, const int ctu_idx)
> > +static int task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint
> *ep, const int ctu_idx)
> > {
> > + if (t->sc) {
> > + // the task already inited, error bitstream
> > + return AVERROR_INVALIDDATA;
> > + }
> > t->sc = sc;
> > t->ep = ep;
> > t->ctu_idx = ctu_idx;
> > +
> > + return 0;
> > }
> >
> > static uint8_t task_add_score(VVCTask *t, const VVCTaskStage stage)
> > @@ -758,24 +764,35 @@ static void submit_entry_point(VVCContext *s,
> VVCFrameThread *ft, SliceContext *
> > frame_thread_add_score(s, ft, t->rx, t->ry, VVC_TASK_STAGE_PARSE);
> > }
> >
> > -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> > +int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> > {
> > VVCFrameThread *ft = fc->ft;
> >
> > - for (int i = 0; i < fc->nb_slices; i++) {
> > - SliceContext *sc = fc->slices[i];
> > - for (int j = 0; j < sc->nb_eps; j++) {
> > - EntryPoint *ep = sc->eps + j;
> > - for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> > - const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> > - VVCTask *t = ft->tasks + rs;
> > -
> > - task_init_parse(t, sc, ep, k);
> > - check_colocation(s, t);
> > + // We'll handle this in two passes:
> > + // Pass 0 to initialize tasks with parser, this will help detect
> bit stream error
> > + // Pass 1 to shedule location check and submit the entry point
> > + for (int pass = 0; pass < 2; pass++) {
> > + for (int i = 0; i < fc->nb_slices; i++) {
> > + SliceContext *sc = fc->slices[i];
> > + for (int j = 0; j < sc->nb_eps; j++) {
> > + EntryPoint *ep = sc->eps + j;
> > + for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> > + const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> > + VVCTask *t = ft->tasks + rs;
> > + if (pass) {
> > + check_colocation(s, t);
> > + } else {
> > + const int ret = task_init_parse(t, sc, ep, k);
> > + if (ret < 0)
> > + return ret;
> > + }
> > + }
> > + if (pass)
> > + submit_entry_point(s, ft, sc, ep);
> > }
> > - submit_entry_point(s, ft, sc, ep);
> > }
> > }
> > + return 0;
> > }
> >
> > int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc)
> > diff --git a/libavcodec/vvc/thread.h b/libavcodec/vvc/thread.h
> > index 55bb4ea244..8ac59b2ecf 100644
> > --- a/libavcodec/vvc/thread.h
> > +++ b/libavcodec/vvc/thread.h
> > @@ -30,7 +30,7 @@ void ff_vvc_executor_free(struct AVExecutor **e);
> >
> > int ff_vvc_frame_thread_init(VVCFrameContext *fc);
> > void ff_vvc_frame_thread_free(VVCFrameContext *fc);
> > -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
> > +int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
> > int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc);
> >
> > #endif // AVCODEC_VVC_THREAD_H
>
> This patch fixes most of the fuzz bitstreams I have which enter infinite
> loops, but also introduces a regression, turning some other bitstreams
> which were okay before into infinite loops.
>
> --
> Frank
> _______________________________________________
> 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] 3+ messages in thread
end of thread, other threads:[~2024-04-25 14:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-21 14:52 [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice Nuo Mi
2024-04-21 16:34 ` Frank Plowman
2024-04-25 14:05 ` Nuo Mi
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