Re: [patch] ses: tighten range checks in ses_intf_add()

[Willy Tarreau <w@1wt.eu>]

Hi Dan,

On Mon, Oct 19, 2015 at 04:48:20PM +0300, Dan Carpenter wrote:
> We test that "type_ptr" is within the buffer but then we read from
> "type_ptr[3]" so we could be reading beyond the end of the buffer.
> 
> Reported-by: "Berry Cheng ??????(??????)" <chengmiao.cj@alibaba-inc.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This isn't a complete fix because we still need more range checking in
> all the other places which use type_ptr like ses_get_page2_descriptor().
> We record len as page1_len but we don't use it anywhere...
> 
> I wonder if someone knew the expected format we could make reject too
> short lengths earlier.
> 
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index dcb0d76..39f69b0 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -641,7 +641,7 @@ static int ses_intf_add(struct device *cdev,
>  	/* begin at the enclosure descriptor */
>  	type_ptr = buf + 8;
>  	/* skip all the enclosure descriptors */
> -	for (i = 0; i < num_enclosures && type_ptr < buf + len; i++) {
> +	for (i = 0; i < num_enclosures && type_ptr + 4 < buf + len; i++) {
>  		types += type_ptr[2];
>  		type_ptr += type_ptr[3] + 4;


why "type_ptr + 4 < buf + len" here ? You're using type_ptr[3] only,
so it's either "type_ptr + 4 <= buf + len" or "type_ptr + 3 < buf + len".

> @@ -649,7 +649,7 @@ static int ses_intf_add(struct device *cdev,
>  	ses_dev->page1_types = type_ptr;
>  	ses_dev->page1_num_types = types;
>  
> -	for (i = 0; i < types && type_ptr < buf + len; i++, type_ptr += 4) {
> +	for (i = 0; i < types && type_ptr + 2 < buf + len; i++, type_ptr += 4) {
>  		if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
>  		    type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE)
>  			components += type_ptr[1];

Same here where I'd expect "type_ptr + 1 < buf + len"

Regards,
Willy