On Wed, 2015-12-09 at 18:07 +0100, Tomas Henzl wrote:
> 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.
Well, it wouldn't be a bug fix and it's strictly not necessary. in
ses_intf_add() we won't attach if the initial retrieve of page 2 fails.
That means we already have an old copy. So if there's a failure in
ses_get_page2_descriptor() then we're just working from old data.
Essentially there's nothing else we could do (except perhaps log the
problem).
James
> -tms
>
> >
> > Reported-by: Andrea Gelmini <andrea.gelmini@xxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> >
> > ---
> >
> > 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 majordomo@xxxxxxxxxxxxxxx
> > 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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
