Tomas Härdin: > tis 2022-09-27 klockan 03:47 +0200 skrev Andreas Rheinhardt: >> Affected the jpeg2000dsp checkasm test. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com> >> --- >> libavcodec/jpeg2000dsp.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/jpeg2000dsp.c b/libavcodec/jpeg2000dsp.c >> index b61be3b72f..b1bff6d5b1 100644 >> --- a/libavcodec/jpeg2000dsp.c >> +++ b/libavcodec/jpeg2000dsp.c >> @@ -76,14 +76,13 @@ static void ict_int(void *_src0, void *_src1, >> void *_src2, int csize) >> >> static void rct_int(void *_src0, void *_src1, void *_src2, int >> csize) >> { >> - int32_t *src0 = _src0, *src1 = _src1, *src2 = _src2; >> - int32_t i0, i1, i2; >> + uint32_t *src0 = _src0, *src1 = _src1, *src2 = _src2; >> int i; >> >> for (i = 0; i < csize; i++) { >> - i1 = *src0 - (*src2 + *src1 >> 2); >> - i0 = i1 + *src2; >> - i2 = i1 + *src1; >> + uint32_t i1 = *src0 - ((int32_t)(*src2 + *src1) >> 2); > > The addition could conceivably overflow. Also could just use / 4 > instead of >> 2.
The addition uses unsigned types, so that overflow is defined. (I now notice that my commit message is slightly confusing in this regard: It uses the spec verbiage which is that arithmetic on unsigned integer types can never overflow, because it is performed modulo the max of said type + 1; but it is nevertheless common to still call this overflow.) Furthermore, the shift is performed on signed types and the rounding for negative numbers divided by four is different than what >> 2 produces (integer division is defined to use rounding towards zero, whereas right shifts of negative numbers are implementation defined and typically use rounding towards -inf (we require this behaviour)). The test fails if I use / 4 here (with or without the cast to int32_t). > >> + int32_t i0 = i1 + *src2; >> + int32_t i2 = i1 + *src1; > > These could also overflow. And agian, not in typical use obviously > because this is for lossless, but for malicious files possibly. > The addition uses unsigned types, so that overflow is defined. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".