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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 28 Sep 2021 14:34:49 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Daniel Palmer <daniel@...f.com>
Cc:     Daniel Palmer <daniel@...ngy.jp>,
        Romain Perier <romain.perier@...il.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Nobuhiro Iwamatsu <iwamatsu@...auri.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-rtc@...r.kernel.org, kernel-janitors@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH][next] rtc: msc313: Fix unintentional sign extension issue
 on left shift of a u16

On 28/09/2021 14:31, Daniel Palmer wrote:
> Hi Colin,
> 
> On Tue, 28 Sept 2021 at 21:39, Colin King <colin.king@...onical.com> wrote:
>> Shifting the u16 value returned by readw by 16 bits to the left
>> will be promoted to a 32 bit signed int and then sign-extended
>> to an unsigned long. If the top bit of the readw is set then
>> the shifted value will be sign extended and the top 32 bits of
>> the result will be set.
> 
> Ah,.. C is fun in all the wrong places. :)
> These chips are full of 32bit registers that are split into two 16
> registers 4 bytes apart when seen from the ARM CPU so we probably have
> this same mistake in a few other places.
> 
> A similar pattern is used a bit later on in the same file to read the counter:
> 
> seconds = readw(priv->rtc_base + REG_RTC_CNT_VAL_L)
> | (readw(priv->rtc_base + REG_RTC_CNT_VAL_H) << 16);

Ah, I missed that one! I'll send a V2.

> 
> I guess it works at the moment because the top bit won't be set until 2038.

I hope to be retired by then, but I guess fixing it up before 2038 is a
good idea ;-)

> 
> Thanks,
> 
> Daniel
> 

Powered by blists - more mailing lists