linux-kernel - RE: [PATCH] Fix chance of sign extension to nsec after its msb is set during calculation.

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:   Sun, 4 Sep 2016 11:01:51 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Liav Rehana <liavr@...lanox.com>
cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "john.stultz@...aro.org" <john.stultz@...aro.org>,
        Noam Camus <noamca@...lanox.com>,
        Elad Kanfi <eladkan@...lanox.com>
Subject: RE: [PATCH] Fix chance of sign extension to nsec after its msb is
 set during calculation.

On Sun, 4 Sep 2016, Liav Rehana wrote:

Please do not top post and trim your reply.

> The root of the problem is that in case the multiplication of delta and
> tkr->mult in the line that I've changed is too big that the MSB of the
> result is set, then the shift will cause an unwanted sign extension.

I completely understand that, but as I said before:

> > This typecast is just a baindaid. What happens if you double the
> > suspend time?  The multiplication will simply overflow. So the proper
> > fix is to sanity check delta and do multiple conversions if delta is
> > big enough.  Preferrably this happens somewhere at the call site and
> > not in this hotpath function.

> That sign extension will be avoided completely if the variable nsec was
> unsigned (u64 instead of s64), so I think the correct solution for this
> is to change the type of nsec to u64.

That's a different story and its not a solution for the general problem of

       delta * mult >= (1 << 31) or delta * mult >= (1 << 32)

Thanks,

	tglx