[{"id":5104,"web_url":"https://patchwork.libcamera.org/comment/5104/","msgid":"<20200607153507.GA5958@pendragon.ideasonboard.com>","date":"2020-06-07T15:35:07","subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: ipa_module: Fix\n\timplicit sign-extension in elfSection","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Sun, Jun 07, 2020 at 02:30:18PM +0000, Umang Jain wrote:\n> Given how the elfSection() is used the sub-expression\n\ns/is used/uses/\n\n> (idx * eHdr->e_shentsize)\n> it has effectively two (16 bits, unsigned) operands.\n> The sub-expression is promoted to type int (32 bits, signed) for\n> multiplication and then added to eHdr->e_shoff, which is uint32_t\n> on 32-bit platforms and uint64_t on 64-bit platforms. Since eHdr->e_shoff\n> is unsigned, the integer conversion rules dictates that the other signed\n> operand(i.e. the resultant of aforementioned sub-expression) will be\n\ns/operand/operand /\ns/resultant/result/\n\n> converted to unsigned type too. This causes sign-extension for both of\n> the above operands to match eHdr->e_shoff's type and should be avoided.\n> \n> The solution is to explicitly cast one of the operands of the\n> sub-expression with unsigned int type. Hence, the other operand will be\n> integer promoted and the resultant will also be of unsigned int type,\n> not requiring to bother about a sign-extension.\n> \n> Reported-by: Coverity CID=280008\n> Reported-by: Coverity CID=280009\n> Reported-by: Coverity CID=280010\n> Signed-off-by: Umang Jain \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\nI'll fix the above when applying. Thanks for fixing this issue !\n\n> ---\n> src/libcamera/ipa_module.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index 60aaa34..72e357e 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -93,7 +93,8 @@ ElfW(Shdr) *elfSection(Span elf, ElfW(Ehdr) *eHdr, ElfW(Half) idx)\n> \tif (idx >= eHdr->e_shnum)\n> \t\treturn nullptr;\n> \n> -\toff_t offset = eHdr->e_shoff + idx * eHdr->e_shentsize;\n> +\toff_t offset = eHdr->e_shoff + idx *\n> +\t\t\t\t static_cast(eHdr->e_shentsize);\n> \treturn elfPointer(elf, offset);\n> }\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D554600F7\n\tfor ;\n\tSun, 7 Jun 2020 17:35:27 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 880902C9;\n\tSun, 7 Jun 2020 17:35:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GjzDdkot\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591544126;\n\tbh=4X1TKz+1XeKizaCXfroBjws1+weXzk5M9Bgdv6Z24aQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GjzDdkotnfzYIcGCKXDs+Z6IW9LXl6y3YRsl2zsHlkhjL9nVMtABMl0JfkFvjQ5i/\n\tuJv57ChdW03wH7AM0eGurXJL87c7eVy6FcUwQevlFq76qBXuG6JKjDALELB767zOuY\n\tjg8C7ka8evn2A0ERNu8tXI0BJEY5XKBde27YTjvY=","Date":"Sun, 7 Jun 2020 18:35:07 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com","Message-ID":"<20200607153507.GA5958@pendragon.ideasonboard.com>","References":"<20200607143012.17752-1-email@uajain.com>\n\t<20200607143012.17752-3-email@uajain.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200607143012.17752-3-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: ipa_module: Fix\n\timplicit sign-extension in elfSection","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 07 Jun 2020 15:35:27 -0000"}},{"id":5110,"web_url":"https://patchwork.libcamera.org/comment/5110/","msgid":"<3990a02b-0f5a-7e25-5664-3d0dcede6752@uajain.com>","date":"2020-06-07T17:12:32","subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: ipa_module: Fix\n\timplicit sign-extension in elfSection","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent and Kieran\n\nOn 6/7/20 9:05 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Sun, Jun 07, 2020 at 02:30:18PM +0000, Umang Jain wrote:\n>> Given how the elfSection() is used the sub-expression\n> s/is used/uses/\n>\n>> (idx * eHdr->e_shentsize)\n>> it has effectively two (16 bits, unsigned) operands.\n>> The sub-expression is promoted to type int (32 bits, signed) for\n>> multiplication and then added to eHdr->e_shoff, which is uint32_t\n>> on 32-bit platforms and uint64_t on 64-bit platforms. Since eHdr->e_shoff\n>> is unsigned, the integer conversion rules dictates that the other signed\n>> operand(i.e. the resultant of aforementioned sub-expression) will be\n> s/operand/operand /\n> s/resultant/result/\n>\n>> converted to unsigned type too. This causes sign-extension for both of\n>> the above operands to match eHdr->e_shoff's type and should be avoided.\n>>\n>> The solution is to explicitly cast one of the operands of the\n>> sub-expression with unsigned int type. Hence, the other operand will be\n>> integer promoted and the resultant will also be of unsigned int type,\n>> not requiring to bother about a sign-extension.\n>>\n>> Reported-by: Coverity CID=280008\n>> Reported-by: Coverity CID=280009\n>> Reported-by: Coverity CID=280010\n>> Signed-off-by: Umang Jain \n>> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n>\n> I'll fix the above when applying. Thanks for fixing this issue !\nThanks for the suggestions, last-min fixups! and thorough reviews. :)\n>> ---\n>> src/libcamera/ipa_module.cpp | 3 ++-\n>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n>> index 60aaa34..72e357e 100644\n>> --- a/src/libcamera/ipa_module.cpp\n>> +++ b/src/libcamera/ipa_module.cpp\n>> @@ -93,7 +93,8 @@ ElfW(Shdr) *elfSection(Span elf, ElfW(Ehdr) *eHdr, ElfW(Half) idx)\n>> \tif (idx >= eHdr->e_shnum)\n>> \t\treturn nullptr;\n>> \n>> -\toff_t offset = eHdr->e_shoff + idx * eHdr->e_shentsize;\n>> +\toff_t offset = eHdr->e_shoff + idx *\n>> +\t\t\t\t static_cast(eHdr->e_shentsize);\n>> \treturn elfPointer(elf, offset);\n>> }\n>>","headers":{"Return-Path":"","Received":["from o1.f.az.sendgrid.net (o1.f.az.sendgrid.net [208.117.55.132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BE85600F7\n\tfor ;\n\tSun, 7 Jun 2020 19:12:34 +0200 (CEST)","by filter0072p3las1.sendgrid.net with SMTP id\n\tfilter0072p3las1-16738-5EDD2000-26\n\t2020-06-07 17:12:32.369156004 +0000 UTC m=+409780.322760462","from mail.uajain.com (unknown)\n\tby ismtpd0005p1maa1.sendgrid.net (SG) with ESMTP\n\tid pS6k-ZXJR5ucjkTJ6MNxTQ Sun, 07 Jun 2020 17:12:31.730 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=uajain.com\n\theader.i=@uajain.com header.b=\"rXLJFwuN\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=uajain.com;\n\th=subject:references:from:mime-version:in-reply-to:to:cc:content-type:\n\tcontent-transfer-encoding;\n\ts=s1; bh=jXJmhmMcNyedyqYURD7qc7qRebUTHfkJAa9hRTcvu9k=;\n\tb=rXLJFwuNxgYzuYqsBVNSNW/m7yEW9W1GEi4LmkBjzBzFS/v4WnY538cvmSA89VQVBC+H\n\t8itcu56Op/rKBKTEN1RbAh33ZMhcfzLpT2E7abnTkR7hJ2+dIyVFZAeKDWnuRJfHGOvCY1\n\tq5cushIWhE5IZ9U1OROVAkAV3hrqmKZ18=","References":"<20200607143012.17752-1-email@uajain.com>\n\t<20200607143012.17752-3-email@uajain.com>\n\t<20200607153507.GA5958@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"<3990a02b-0f5a-7e25-5664-3d0dcede6752@uajain.com>","Date":"Sun, 07 Jun 2020 17:12:32 +0000 (UTC)","Mime-Version":"1.0","In-Reply-To":"<20200607153507.GA5958@pendragon.ideasonboard.com>","X-SG-EID":"1Q40EQ7YGir8a9gjSIAdTjhngY657NMk9ckeo4dbHZDiOpywc/L3L9rFqlwE4KPc/mSirh79xX0l6HRHZb2eYFVcbxpI67D4lx+b9S6HrVPliyg9rx0RrrhSi4flbUoIQY+8m+gvm2bsXLquNpWv3724Wo+pVEggl5IITH/T6rPf4ImFgGmyAxhYAIvQOTdBty3vK76ENLZwL4jyrEvHunar59eHjA72Of4i0GvmWVl7pJ1qKhXMPyVAqjFid2Fw","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, kieran.bingham@ideasonboard.com,\n\tkieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=us-ascii; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: ipa_module: Fix\n\timplicit sign-extension in elfSection","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 07 Jun 2020 17:12:35 -0000"}}]