From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id EC90F4352E for ; Thu, 16 Jun 2022 10:22:51 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D6DA168B7E7; Thu, 16 Jun 2022 13:22:48 +0300 (EEST) Received: from shout01.mail.de (shout01.mail.de [62.201.172.24]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7CC3B68B4BF for ; Thu, 16 Jun 2022 13:22:42 +0300 (EEST) Received: from postfix02.mail.de (postfix02.bt.mail.de [10.0.121.126]) by shout01.mail.de (Postfix) with ESMTP id 0A0E1A0BAB for ; Thu, 16 Jun 2022 12:22:42 +0200 (CEST) Received: from smtp02.mail.de (smtp02.bt.mail.de [10.0.121.212]) by postfix02.mail.de (Postfix) with ESMTP id E6A42A03C2 for ; Thu, 16 Jun 2022 12:22:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=mail.de; s=mailde202009; t=1655374961; bh=hzhekvwcTVx5qEFnS3h8bXPanFu3OqEYNmDgst4ceSc=; h=Message-ID:Date:Subject:To:From:From:To:CC:Subject:Reply-To; b=lYI2AiRpF7k8oybRwzTyvuTqoQJ5DALOclu8GZScZnXYLcf+p1HIVp5hUkLOm6iZV G/sogpHNlryRwBRW4e1IG/KT4MaRE39m2Htvje3XV/uMKbCcdHNc0CIfh0IA/IL4R6 35IURYAF9JYSuUZ9KwdktkD+iUPXxIC3WNC0nwELONFKHrIk/XP02dH0Vk4bB/aTbv CrBDmOVFMtKyGef9vJqIU3Qk1U7i8j7rYqH8CX8smMoP9kmKQU6n7CzQ3bziNT2ms6 PcR18q4qq8tzvAdmBwH7slueMeZl2S3D6jB4CQgHoD0leKOCGWQwpT0z/Rt/5WeKMt FwDgjvCUgdW8A== Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by smtp02.mail.de (Postfix) with ESMTPSA id B9BAAA0AD8 for ; Thu, 16 Jun 2022 12:22:41 +0200 (CEST) Content-Type: multipart/mixed; boundary="------------n0Sn8peVhRJPmyfmDT0d8dWK" Message-ID: <82d9a6ac-5365-dae4-9825-eabec6b2151f@mail.de> Date: Thu, 16 Jun 2022 12:22:40 +0200 MIME-Version: 1.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <80816607-3610-9133-9d5d-7fee3040de06@mail.de> From: Thilo Borgmann In-Reply-To: X-purgate: clean X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate-type: clean X-purgate-Ad: Categorized by eleven eXpurgate (R) http://www.eleven.de X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate: clean X-purgate-size: 4254 X-purgate-ID: 154282::1655374961-0000737C-3E37B747/0/0 Subject: Re: [FFmpeg-devel] [PATCH] lavc/dovi_rpu: Fix UB for possible left shift of negative values X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: This is a multi-part message in MIME format. --------------n0Sn8peVhRJPmyfmDT0d8dWK Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Am 16.06.22 um 12:13 schrieb Andreas Rheinhardt: > Thilo Borgmann: >> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c >> index a87562c8a3..833ce9e705 100644 >> --- a/libavcodec/dovi_rpu.c >> +++ b/libavcodec/dovi_rpu.c >> @@ -153,7 +153,7 @@ static inline uint64_t get_ue_coef(GetBitContext *gb, const AVDOVIRpuDataHeader >> case RPU_COEFF_FIXED: >> ipart = get_ue_golomb_long(gb); >> fpart.u32 = get_bits_long(gb, hdr->coef_log2_denom); >> - return (ipart << hdr->coef_log2_denom) + fpart.u32; >> + return ipart * (1 << hdr->coef_log2_denom) + fpart.u32; >> >> case RPU_COEFF_FLOAT: >> fpart.u32 = get_bits_long(gb, 32); >> @@ -172,7 +172,7 @@ static inline int64_t get_se_coef(GetBitContext *gb, const AVDOVIRpuDataHeader * >> case RPU_COEFF_FIXED: >> ipart = get_se_golomb_long(gb); >> fpart.u32 = get_bits_long(gb, hdr->coef_log2_denom); >> - return (ipart << hdr->coef_log2_denom) + fpart.u32; >> + return ipart * (1 << hdr->coef_log2_denom) + fpart.u32; >> >> case RPU_COEFF_FLOAT: >> fpart.u32 = get_bits_long(gb, 32); > > coef_log2_denom can be in the range 13..32. This means that 1 << > hdr->coef_log2_denom can be UB (namely if coef_log2_denom is 31 or 32 > for ordinary 32 bit ints); this time it is not UB that happens to work > as expected, because 1 << 32 will be 0 or 1 (depending upon the system) > and not 2^32. In case of get_ue_coef() this actually adds UB to > otherwise fine code. So 1LL it needs to be, not? Am I still missing something? V3 attached. Thanks, Thilo --------------n0Sn8peVhRJPmyfmDT0d8dWK Content-Type: text/plain; charset=UTF-8; name="v3-0001-lavc-dovi_rpu-Fix-UB-for-possible-left-shift-of-n.patch" Content-Disposition: attachment; filename*0="v3-0001-lavc-dovi_rpu-Fix-UB-for-possible-left-shift-of-n.pa"; filename*1="tch" Content-Transfer-Encoding: base64 RnJvbSBjM2U0Mjk3OTgzZjhhYTc0YjczZGYxZWJmNDhhNDliM2NlYTczNmE3IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBNaWNoYWVsIEdvdWxldCA8bWdvdWxldEBmYi5jb20+ CkRhdGU6IFRodSwgMTYgSnVuIDIwMjIgMTI6MjA6MTAgKzAyMDAKU3ViamVjdDogW1BBVENI IHYzXSBsYXZjL2RvdmlfcnB1OiBGaXggVUIgZm9yIHBvc3NpYmxlIGxlZnQgc2hpZnQgb2Yg bmVnYXRpdmUKIHZhbHVlcwoKSXQgaXMgdW5kZWZpbmVkIHRvIGxlZnQtc2hpZnQgYSBuZWdh dGl2ZSB2YWx1ZS4KQ2hhbmdlZCBnZXRfdWVfY29lZigpIGFzIHdlbGwgZm9yIGNvbnNpc3Rl bmN5IGFsdGhvdWdoIGl0cyBhbGwgcG9zaXRpdmUgdmFsdWVzIHRoZXJlLgotLS0KIGxpYmF2 Y29kZWMvZG92aV9ycHUuYyB8IDQgKystLQogMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9u cygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9saWJhdmNvZGVjL2RvdmlfcnB1 LmMgYi9saWJhdmNvZGVjL2RvdmlfcnB1LmMKaW5kZXggYTg3NTYyYzhhMy4uYWIxNDM5N2Vi ZCAxMDA2NDQKLS0tIGEvbGliYXZjb2RlYy9kb3ZpX3JwdS5jCisrKyBiL2xpYmF2Y29kZWMv ZG92aV9ycHUuYwpAQCAtMTUzLDcgKzE1Myw3IEBAIHN0YXRpYyBpbmxpbmUgdWludDY0X3Qg Z2V0X3VlX2NvZWYoR2V0Qml0Q29udGV4dCAqZ2IsIGNvbnN0IEFWRE9WSVJwdURhdGFIZWFk ZXIKICAgICBjYXNlIFJQVV9DT0VGRl9GSVhFRDoKICAgICAgICAgaXBhcnQgPSBnZXRfdWVf Z29sb21iX2xvbmcoZ2IpOwogICAgICAgICBmcGFydC51MzIgPSBnZXRfYml0c19sb25nKGdi LCBoZHItPmNvZWZfbG9nMl9kZW5vbSk7Ci0gICAgICAgIHJldHVybiAoaXBhcnQgPDwgaGRy LT5jb2VmX2xvZzJfZGVub20pICsgZnBhcnQudTMyOworICAgICAgICByZXR1cm4gaXBhcnQg KiAoMUxMIDw8IGhkci0+Y29lZl9sb2cyX2Rlbm9tKSArIGZwYXJ0LnUzMjsKIAogICAgIGNh c2UgUlBVX0NPRUZGX0ZMT0FUOgogICAgICAgICBmcGFydC51MzIgPSBnZXRfYml0c19sb25n KGdiLCAzMik7CkBAIC0xNzIsNyArMTcyLDcgQEAgc3RhdGljIGlubGluZSBpbnQ2NF90IGdl dF9zZV9jb2VmKEdldEJpdENvbnRleHQgKmdiLCBjb25zdCBBVkRPVklScHVEYXRhSGVhZGVy ICoKICAgICBjYXNlIFJQVV9DT0VGRl9GSVhFRDoKICAgICAgICAgaXBhcnQgPSBnZXRfc2Vf Z29sb21iX2xvbmcoZ2IpOwogICAgICAgICBmcGFydC51MzIgPSBnZXRfYml0c19sb25nKGdi LCBoZHItPmNvZWZfbG9nMl9kZW5vbSk7Ci0gICAgICAgIHJldHVybiAoaXBhcnQgPDwgaGRy LT5jb2VmX2xvZzJfZGVub20pICsgZnBhcnQudTMyOworICAgICAgICByZXR1cm4gaXBhcnQg KiAoMUxMIDw8IGhkci0+Y29lZl9sb2cyX2Rlbm9tKSArIGZwYXJ0LnUzMjsKIAogICAgIGNh c2UgUlBVX0NPRUZGX0ZMT0FUOgogICAgICAgICBmcGFydC51MzIgPSBnZXRfYml0c19sb25n KGdiLCAzMik7Ci0tIAoyLjIwLjEgKEFwcGxlIEdpdC0xMTcpCgo= --------------n0Sn8peVhRJPmyfmDT0d8dWK Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --------------n0Sn8peVhRJPmyfmDT0d8dWK--