On 8.12.2015 18:00, James Bottomley wrote:
> Simple enclosure implementations (mostly USB) are allowed to return only
> page 8 to every diagnostic query. That really confuses our
> implementation because we assume the return is the page we asked for and
> end up doing incorrect offsets based on bogus information leading to
> accesses outside of allocated ranges. Fix that by checking the page
> code of the return and giving an error if it isn't the one we asked for.
> This should fix reported bugs with USB storage by simply refusing to
> attach to enclosures that behave like this. It's also good defensive
> practise now that we're starting to see more USB enclosures.
Ideally this patch also fixes all callers so they evaluate the return value
from ses_recv_diag. That is missed in ses_enclosure_data_process
and ses_get_page2_descriptor.
-tms
>
> Reported-by: Andrea Gelmini <andrea.gelm...@gelma.net>
> Cc: sta...@vger.kernel.org
> Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
>
> ---
>
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index dcb0d76..7d9cec5 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -84,6 +84,7 @@ static void init_device_slot_control(unsigned char
> *dest_desc,
> static int ses_recv_diag(struct scsi_device *sdev, int page_code,
> void *buf, int bufflen)
> {
> + int ret;
> unsigned char cmd[] = {
> RECEIVE_DIAGNOSTIC,
> 1, /* Set PCV bit */
> @@ -92,9 +93,26 @@ static int ses_recv_diag(struct scsi_device *sdev, int
> page_code,
> bufflen & 0xff,
> 0
> };
> + unsigned char recv_page_code;
>
> - return scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> + ret = scsi_execute_req(sdev, cmd, DMA_FROM_DEVICE, buf, bufflen,
> NULL, SES_TIMEOUT, SES_RETRIES, NULL);
> + if (unlikely(!ret))
> + return ret;
> +
> + recv_page_code = ((unsigned char *)buf)[0];
> +
> + if (likely(recv_page_code == page_code))
> + return ret;
> +
> + /* successful diagnostic but wrong page code. This happens to some
> + * USB devices, just print a message and pretend there was an error */
> +
> + sdev_printk(KERN_ERR, sdev,
> + "Wrong diagnostic page; asked for %d got %u\n",
> + page_code, recv_page_code);
> +
> + return -EINVAL;
> }
>
> static int ses_send_diag(struct scsi_device *sdev, int page_code,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
