linux-kernel - Re: [PATCH 1/8] firmware: Sigma: Prevent out of bounds memory access

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: <201111241226.23405.vapier@gentoo.org>
Date:	Thu, 24 Nov 2011 12:26:21 -0500
From:	Mike Frysinger <vapier@...too.org>
To:	"Lars-Peter Clausen" <lars@...afoo.de>
Cc:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Liam Girdwood <lrg@...com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
	drivers@...log.com, stable@...nel.org
Subject: Re: [PATCH 1/8] firmware: Sigma: Prevent out of bounds memory access

On Thursday 24 November 2011 07:48:20 Lars-Peter Clausen wrote:
> The SigmaDSP firmware loader currently does not perform enough boundary
> size checks when processing the firmware. As a result it is possible that
> a malformed firmware can cause an out of bounds memory access.
> 
> This patch adds checks which ensure that both the action header and the
> payload are completely inside the firmware data boundaries before
> processing them.

in general this looks fine ...

> --- a/drivers/firmware/sigma.c
> +++ b/drivers/firmware/sigma.c
> 
> -/* Return: 0==OK, <0==error, =1 ==no more actions */
>  static int
> +process_sigma_action(struct i2c_client *client, struct
> sigma_action *sa)

looks like you're inverting the semantics of this func.  i'd add an updated 
comment above the func to document the new return values.

> +	/* Reject too small or unreasonable large files. The upper limit is
> +	 * chosen a bit arbitrarily but it should be enough for all practical
> +	 * purposes and having the limit makes it easier to avoid integer
> +	 * overflows later in the loading process. */

multi-line comment style:
	/*
	 * line one
	 * line two
	 */
-mike

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