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

From: Mike Frysinger
Date: Thu Nov 24 2011 - 12:26:13 EST


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: signature.asc
Description: This is a digitally signed message part.