Re: [PATCH 6/7] media: don't do an unsigned int with a 31 bit shift

From: Kees Cook <keescook@chromium.org>
To: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	Andy Walls <awalls@md.metrocast.net>,
	Bluecherry Maintainers <maintainers@bluecherrydvr.com>,
	Anton Sviridenko <anton@corp.bluecherry.net>,
	Andrey Utkin <andrey.utkin@corp.bluecherry.net>,
	Ismael Luceno <ismael@iodev.co.uk>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Andrzej Pietrasiewicz <andrzejtp2010@gmail.com>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Kamil Debski <kamil@wypas.org>,
	Jeongtae Park <jtp.park@samsung.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Hans Verkuil <hverkuil@xs4all.nl>, Antti Palosaari <crope@iki.fi>,
	Mike Isely <isely@pobox.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Richard Fontana <rfontana@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	Colin Ian King <colin.king@canonical.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Steve Longerbeam <slongerbeam@gmail.com>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Tomasz Figa <tfiga@chromium.org>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 6/7] media: don't do an unsigned int with a 31 bit shift
Date: Thu, 29 Aug 2019 10:03:09 -0700	[thread overview]
Message-ID: <201908291002.2F67F5ADA@keescook> (raw)
In-Reply-To: <1a78a757b37d2628312e1d56d7a741ba89d42a91.1566502743.git.mchehab+samsung@kernel.org>

On Thu, Aug 22, 2019 at 04:39:33PM -0300, Mauro Carvalho Chehab wrote:
> Doing something like:
> 
> 	i32 foo = 1, bar;
> 
> 	bar = foo << 31;
> 
> has an undefined behavior in C, as warned by cppcheck, as we're
> shifting a signed integer.
> 
> Instead, force the numbers to be unsigned, in order to solve this
> issue.

I also recommend using the BIT() macro, which does the ULing correctly,
etc.

i.e. instead of:

-	keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+	keyup = (gpio & ir->mask_keyup) ? 1UL << 31 : 0;

use:

-	keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0;
+	keyup = (gpio & ir->mask_keyup) ? BIT(31) : 0;

-- 
Kees Cook