linux-kernel - Re: [PATCH] regmap: Add function check before called format

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]

Message-ID: <20150721172550.GY11162@sirena.org.uk>
Date:	Tue, 21 Jul 2015 18:25:50 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Henry Chen <HenryC.Chen@...iatek.com>
Cc:	Matthias Brugger <matthias.bgg@...il.com>,
	Sascha Hauer <kernel@...gutronix.de>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-mediatek@...ts.infradead.org, eddie.huang@...iatek.com
Subject: Re: [PATCH] regmap: Add function check before called format_val

On Tue, Jul 21, 2015 at 02:07:25PM +0800, Henry Chen wrote:

> Then in driver rtc-mt6397.c, it used regmap_bulk_read() to get the time
> of PMIC, and hit the null function of format_val(), because the
> regmap_bus was null.

> It skipped the initialization of format_val() because bus == null, but
> called the format_val() at regmap_bulk_read() if bus == null.

OK, so the issue here is that when we fall back to regmap_read() we may
do so because we have reg_read() and reg_write() functions which in turn
imply no formatting.  The expectation here is that val must be an array
of int.  The code doesn't completely take that into account though and
the user you're pointing at is assuming it's an array of 16 bit values
which isn't totally unreasonable if it did specify val_bits (we don't
check for that).

> Maybe it was not the good fix for this, but should be a problem need to
> be reported, or should I need to give the regmap_bus on mtk_pmic_wrap.c?

That file isn't in mainline...

memcpy() is definitely not a safe way to move from an unsigned int to a
u16 which is what your specific use case is trying to do.  I'll need to
do an audit of existing users (or someone else will!) to figure out what
people are doing with .val_bits in drivers using reg_read() and
reg_write() but I think what we should be doing here is probably
providing appropriate conversion functions based on val_bits on init.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)