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

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1.1: Type: Text/Plain, Size: 1197 bytes --]

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

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]