From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com ([210.118.77.12]:33722 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756062AbaJXKQe (ORCPT ); Fri, 24 Oct 2014 06:16:34 -0400 Message-id: <544A26FD.9060406@samsung.com> Date: Fri, 24 Oct 2014 14:16:29 +0400 From: Andrey Ryabinin MIME-version: 1.0 Subject: Re: drivers: random: Shift out-of-bounds in _mix_pool_bytes References: <1413802499-17928-1-git-send-email-a.ryabinin@samsung.com> <5444EBFA.5030103@samsung.com> <20141020124929.GA23177@thunk.org> <54451501.2070700@samsung.com> <5445179A.4080804@redhat.com> <20141020141635.GA4499@thunk.org> <20141024100108.GF12706@worktop.programming.kicks-ass.net> In-reply-to: <20141024100108.GF12706@worktop.programming.kicks-ass.net> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit Sender: linux-kbuild-owner@vger.kernel.org List-ID: To: Peter Zijlstra , Theodore Ts'o , Daniel Borkmann , Andrew Morton , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , Sasha Levin , x86@kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, Andreas Dilger , Dmitry Vyukov , Konstantin Khlebnikov On 10/24/2014 02:01 PM, Peter Zijlstra wrote: > On Mon, Oct 20, 2014 at 10:16:35AM -0400, Theodore Ts'o wrote: >> On Mon, Oct 20, 2014 at 04:09:30PM +0200, Daniel Borkmann wrote: >>>> >>>> It's triggering when input_rotate == 0, so UBSan complains about right shift in rol32() >>>> >>>> static inline __u32 rol32(__u32 word, unsigned int shift) >>>> { >>>> return (word << shift) | (word >> (32 - shift)); >>>> } >>> >>> So that would be the case when the entropy store's input_rotate calls >>> _mix_pool_bytes() for the very first time ... I don't think it's an >>> issue though. >> >> I'm sure it's not an issue, but it's still true that >> >> return (word << 0) | (word >> 32); >> >> is technically not undefined, and while it would be unfortunate (and >> highly unlikely) if gcc were to say, start nethack, it's technically >> allowed by the C spec. :-) > > In fact, n >> 32 == n. > > #include > > int main(int argc, char **argv) > { > int i = atoi(argv[1]); > int shift = atoi(argv[2]); > printf("%x\n", i >> shift); > return 0; > } > > $ ./shift 5 32 > 5 > > On x86 at least the shift ops simply mask out the upper bits and > therefore the 32 == 0. > > So you end up OR-ing the same value twice, which is harmless. > > So no misbehaviour on the rol32() function. > E.g. on arm (i >> 32) == 0, so rol32() will also work as expected. But what about other architectures?