| Message ID | 20190918032607.11774-3-andreas.rheinhardt@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 18/09/2019 04:26, Andreas Rheinhardt wrote: > 1 << w is undefined as soon as w is >= 31, as 1 has type int. In the > case of cbs_av1_read_ns, w could potentially even be 32, so one has to > use a 64bit type. > > (None of the current callers ever use arguments that are so large that > the above scenario can happen, but this might change in the future.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/cbs_av1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c > index 98cd37ef74..0ff6d60ae2 100644 > --- a/libavcodec/cbs_av1.c > +++ b/libavcodec/cbs_av1.c > @@ -220,7 +220,7 @@ static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc, > position = get_bits_count(gbc); > > w = av_log2(n) + 1; > - m = (1 << w) - n; > + m = (1LL << w) - n; > > if (get_bits_left(gbc) < w) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid non-symmetric value at " > Seems like it would be better to just assert that n < 2^25 (or something like that) above instead? The 64-bit intermediate with 32-bit variables isn't very nice, and you're also going to run into constraints on get_bits() later as well. Values like this can't be encountered in the current AV1 specification, so the problem is very theoretical in any case. (Patches 1 and 4 from this set LGTM and applied.) Thanks, - Mark
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index 98cd37ef74..0ff6d60ae2 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -220,7 +220,7 @@ static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc, position = get_bits_count(gbc); w = av_log2(n) + 1; - m = (1 << w) - n; + m = (1LL << w) - n; if (get_bits_left(gbc) < w) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid non-symmetric value at "
1 << w is undefined as soon as w is >= 31, as 1 has type int. In the case of cbs_av1_read_ns, w could potentially even be 32, so one has to use a 64bit type. (None of the current callers ever use arguments that are so large that the above scenario can happen, but this might change in the future.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/cbs_av1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)