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@xxxxxxxxxxxxxxx>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
