| Message ID | 20210423134800.724-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | bc2726969400e1e57d25d1042f860eb2cbdf7465 |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/af_adelay: make per channel delay argument an int64_t | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
James Almer: > Should fix ticket #9196 > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavfilter/af_adelay.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c > index 6ac81c2a3e..81ff7947f5 100644 > --- a/libavfilter/af_adelay.c > +++ b/libavfilter/af_adelay.c > @@ -28,9 +28,9 @@ > #include "internal.h" > > typedef struct ChanDelay { > - int delay; > - unsigned delay_index; > - unsigned index; > + int64_t delay; > + size_t delay_index; > + size_t index; > uint8_t *samples; > } ChanDelay; > > @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink) > > p = NULL; > > - ret = av_sscanf(arg, "%d%c", &d->delay, &type); > + ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type); > if (ret != 2 || type != 'S') { > div = type == 's' ? 1.0 : 1000.0; > if (av_sscanf(arg, "%f", &delay) != 1) { > @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink) > if (!d->delay) > continue; > > + if (d->delay > SIZE_MAX) { Does this give a new compiler warning on 64bit systems (where this check is tautologically false)? > + av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n"); > + return AVERROR(EINVAL); > + } > + > d->samples = av_malloc_array(d->delay, s->block_align); > if (!d->samples) > return AVERROR(ENOMEM); >
On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote: > James Almer: >> Should fix ticket #9196 >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavfilter/af_adelay.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c >> index 6ac81c2a3e..81ff7947f5 100644 >> --- a/libavfilter/af_adelay.c >> +++ b/libavfilter/af_adelay.c >> @@ -28,9 +28,9 @@ >> #include "internal.h" >> >> typedef struct ChanDelay { >> - int delay; >> - unsigned delay_index; >> - unsigned index; >> + int64_t delay; >> + size_t delay_index; >> + size_t index; >> uint8_t *samples; >> } ChanDelay; >> >> @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink) >> >> p = NULL; >> >> - ret = av_sscanf(arg, "%d%c", &d->delay, &type); >> + ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type); >> if (ret != 2 || type != 'S') { >> div = type == 's' ? 1.0 : 1000.0; >> if (av_sscanf(arg, "%f", &delay) != 1) { >> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink) >> if (!d->delay) >> continue; >> >> + if (d->delay > SIZE_MAX) { > > Does this give a new compiler warning on 64bit systems (where this check > is tautologically false)? Not here with GCC 10.2.0 mingw-w64, at least. With -Wextra it prints a -Wsign-compare warning, but it gets buried in a dozen other similar warnings. Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align" instead? > >> + av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n"); >> + return AVERROR(EINVAL); >> + } >> + >> d->samples = av_malloc_array(d->delay, s->block_align); >> if (!d->samples) >> return AVERROR(ENOMEM); >> > > _______________________________________________ > 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". >
James Almer: > On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote: >> James Almer: >>> Should fix ticket #9196 >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavfilter/af_adelay.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c >>> index 6ac81c2a3e..81ff7947f5 100644 >>> --- a/libavfilter/af_adelay.c >>> +++ b/libavfilter/af_adelay.c >>> @@ -28,9 +28,9 @@ >>> #include "internal.h" >>> typedef struct ChanDelay { >>> - int delay; >>> - unsigned delay_index; >>> - unsigned index; >>> + int64_t delay; >>> + size_t delay_index; >>> + size_t index; >>> uint8_t *samples; >>> } ChanDelay; >>> @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink) >>> p = NULL; >>> - ret = av_sscanf(arg, "%d%c", &d->delay, &type); >>> + ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type); >>> if (ret != 2 || type != 'S') { >>> div = type == 's' ? 1.0 : 1000.0; >>> if (av_sscanf(arg, "%f", &delay) != 1) { >>> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink) >>> if (!d->delay) >>> continue; >>> + if (d->delay > SIZE_MAX) { >> >> Does this give a new compiler warning on 64bit systems (where this check >> is tautologically false)? > > Not here with GCC 10.2.0 mingw-w64, at least. > With -Wextra it prints a -Wsign-compare warning, but it gets buried in a > dozen other similar warnings. > > Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align" > instead? > Of course not. Then the compiler could not optimize the check (and the string) away. >> >>> + av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n"); >>> + return AVERROR(EINVAL); >>> + } >>> + >>> d->samples = av_malloc_array(d->delay, s->block_align); >>> if (!d->samples) >>> return AVERROR(ENOMEM); >>>
Please apply. On Fri, Apr 23, 2021 at 4:39 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > James Almer: > > On 4/23/2021 11:18 AM, Andreas Rheinhardt wrote: > >> James Almer: > >>> Should fix ticket #9196 > >>> > >>> Signed-off-by: James Almer <jamrial@gmail.com> > >>> --- > >>> libavfilter/af_adelay.c | 13 +++++++++---- > >>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c > >>> index 6ac81c2a3e..81ff7947f5 100644 > >>> --- a/libavfilter/af_adelay.c > >>> +++ b/libavfilter/af_adelay.c > >>> @@ -28,9 +28,9 @@ > >>> #include "internal.h" > >>> typedef struct ChanDelay { > >>> - int delay; > >>> - unsigned delay_index; > >>> - unsigned index; > >>> + int64_t delay; > >>> + size_t delay_index; > >>> + size_t index; > >>> uint8_t *samples; > >>> } ChanDelay; > >>> @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink) > >>> p = NULL; > >>> - ret = av_sscanf(arg, "%d%c", &d->delay, &type); > >>> + ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type); > >>> if (ret != 2 || type != 'S') { > >>> div = type == 's' ? 1.0 : 1000.0; > >>> if (av_sscanf(arg, "%f", &delay) != 1) { > >>> @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink) > >>> if (!d->delay) > >>> continue; > >>> + if (d->delay > SIZE_MAX) { > >> > >> Does this give a new compiler warning on 64bit systems (where this check > >> is tautologically false)? > > > > Not here with GCC 10.2.0 mingw-w64, at least. > > With -Wextra it prints a -Wsign-compare warning, but it gets buried in a > > dozen other similar warnings. > > > > Do you prefer if i make the check "d->delay > SIZE_MAX / s->block_align" > > instead? > > > > Of course not. Then the compiler could not optimize the check (and the > string) away. > > >> > >>> + av_log(ctx, AV_LOG_ERROR, "Requested delay is too > big.\n"); > >>> + return AVERROR(EINVAL); > >>> + } > >>> + > >>> d->samples = av_malloc_array(d->delay, s->block_align); > >>> if (!d->samples) > >>> return AVERROR(ENOMEM); > >>> > _______________________________________________ > 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". >
On 4/25/2021 6:06 AM, Paul B Mahol wrote:
> Please apply.
Applied it earlier today, but forgot to reply, sorry.
diff --git a/libavfilter/af_adelay.c b/libavfilter/af_adelay.c index 6ac81c2a3e..81ff7947f5 100644 --- a/libavfilter/af_adelay.c +++ b/libavfilter/af_adelay.c @@ -28,9 +28,9 @@ #include "internal.h" typedef struct ChanDelay { - int delay; - unsigned delay_index; - unsigned index; + int64_t delay; + size_t delay_index; + size_t index; uint8_t *samples; } ChanDelay; @@ -152,7 +152,7 @@ static int config_input(AVFilterLink *inlink) p = NULL; - ret = av_sscanf(arg, "%d%c", &d->delay, &type); + ret = av_sscanf(arg, "%"SCNd64"%c", &d->delay, &type); if (ret != 2 || type != 'S') { div = type == 's' ? 1.0 : 1000.0; if (av_sscanf(arg, "%f", &delay) != 1) { @@ -194,6 +194,11 @@ static int config_input(AVFilterLink *inlink) if (!d->delay) continue; + if (d->delay > SIZE_MAX) { + av_log(ctx, AV_LOG_ERROR, "Requested delay is too big.\n"); + return AVERROR(EINVAL); + } + d->samples = av_malloc_array(d->delay, s->block_align); if (!d->samples) return AVERROR(ENOMEM);
Should fix ticket #9196 Signed-off-by: James Almer <jamrial@gmail.com> --- libavfilter/af_adelay.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)