linux-kernel - Re: [EXT] RE: [PATCH] media: amphion: fix some error related with undefined reference to _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3021738c-06e4-6760-5a70-5b3dc57f1e96@arm.com>
Date:   Thu, 10 Mar 2022 11:31:41 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Ming Qian <ming.qian@....com>,
        David Laight <David.Laight@...LAB.COM>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>
Cc:     "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [EXT] RE: [PATCH] media: amphion: fix some error related with
 undefined reference to __divdi3

On 2022-03-10 08:36, Ming Qian wrote:
>> -----Original Message-----
>> From: David Laight [mailto:David.Laight@...LAB.COM]
>> Sent: Wednesday, March 9, 2022 9:27 PM
>> To: Ming Qian <ming.qian@....com>; mchehab@...nel.org;
>> shawnguo@...nel.org; robh+dt@...nel.org; s.hauer@...gutronix.de
>> Cc: hverkuil-cisco@...all.nl; kernel@...gutronix.de; festevam@...il.com;
>> dl-linux-imx <linux-imx@....com>; Aisheng Dong <aisheng.dong@....com>;
>> linux-media@...r.kernel.org; linux-kernel@...r.kernel.org;
>> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org
>> Subject: [EXT] RE: [PATCH] media: amphion: fix some error related with
>> undefined reference to __divdi3
>>
>> Caution: EXT Email
>>
>> From: Ming Qian
>>> Sent: 09 March 2022 05:02
>> ...
>>> 3. use 'val >> 1' instead of ' val / 2'
>>
>> The compiler should do that anyway.
>>
>> Especially for unsigned values.
>> And it has the wrong (different) rounding for negative values.
>>
>>          David
>>
> 
> Hi David,
>      Yes, you are right, if the value is negative, the behavior is wrong.
>      But here, the value type is u32, so I think it's OK.

Well, it depends on the semantic intent, really. If you're packing a 
bitfield which encodes bits 31:1 of some value then a shift is the most 
appropriate operation. However if you're literally calculating half of a 
value for, say, a 50% threshold level, or the number of 16-bit words 
represented by a byte length, then semantically it's a division, so it 
should use the divide operator rather than obfuscating it behind a 
shift. Constant division is something that even the most basic 
optimising compiler should handle with ease.

One more thing that's not the fault of this patch, but stood out in the 
context:

@@ -1566,7 +1568,7 @@ static bool vpu_malone_check_ready(struct 
vpu_shared_addr *shared, u32 instance)
  	u32 wptr = desc->wptr;
  	u32 used = (wptr + size - rptr) % size;

-	if (!size || used < size / 2)
+	if (!size || used < (size >> 1))
  		return true;

  	return false;

That's not safe: if "size" is 0 then the undefined behaviour has already 
happened before the "!size" check is reached. If "size" really can be 0, 
then it needs to be checked *before* it is used as a divisor to 
calculate "used".

Robin.