On Tue, 2019-01-22 at 23:49 -0500, Douglas Gilbert wrote:
> On 2019-01-22 7:56 p.m., Bart Van Assche wrote:
> > On Tue, 2019-01-22 at 18:30 -0500, Douglas Gilbert wrote:
> > > This patchset needs something like the following if UAS (USB Attached
> > > SCSI) is configured in your kernel.
> > >
> > > Beware of tabs/spaces/line_wraps as this is a cut and paste:
> > >
> > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> > > index 36742e8e7edc..24f3f95917a5 100644
> > > --- a/drivers/usb/storage/uas.c
> > > +++ b/drivers/usb/storage/uas.c
> > > @@ -401,9 +401,9 @@ static void uas_data_cmplt(struct urb *urb)
> > > if (status != -ENOENT && status != -ECONNRESET &&
> > > status !=
> > > -ESHUTDOWN)
> > > uas_log_cmd_state(cmnd, "data cmplt err",
> > > status);
> > > /* error: no data transfered */
> > > - sdb->resid = sdb->length;
> > > + scsi_in_set_resid(cmnd, sdb->length);
> > > } else {
> > > - sdb->resid = sdb->length - urb->actual_length;
> > > + scsi_in_set_resid(cmnd, sdb->length - urb->actual_length);
> > > }
> > > uas_try_complete(cmnd, __func__);
> > > out:
> >
> > Thanks Doug! I will fold a slightly modified version of this patch in. BTW,
> > does this mean that you have been able to test this patch series?
>
> Yes, but I didn't get far. With my sg v4 driver, scsi_debug and my sg_tst_bidi
> utility and this patchset after a short while I get a NULL pointer dereference
> in scsi_mq_prep_fn() inside the bidi conditional, probably the memset().
>
> RIP ---> 0x2ce7
>
> ....
> if (blk_bidi_rq(req)) {
> 2c9a: 49 8b 85 20 01 00 00 mov 0x120(%r13),%rax
> 2ca1: 48 85 c0 test %rax,%rax
> 2ca4: 74 60 je 2d06 <scsi_queue_rq+0x436>
> memset(&scsi_in_cmd(cmd)->sdb, 0,
> 2ca6: 48 c7 80 28 02 00 00 movq $0x0,0x228(%rax)
> 2cad: 00 00 00 00
> 2cb1: 48 c7 80 30 02 00 00 movq $0x0,0x230(%rax)
> 2cb8: 00 00 00 00
> 2cbc: 48 c7 80 38 02 00 00 movq $0x0,0x238(%rax)
> 2cc3: 00 00 00 00
> 2cc7: 49 8b 85 60 02 00 00 mov 0x260(%r13),%rax
> 2cce: 48 8b 90 20 01 00 00 mov 0x120(%rax),%rdx
> 2cd5: 48 8d 82 28 01 00 00 lea 0x128(%rdx),%rax
> 2cdc: 48 85 d2 test %rdx,%rdx
> 2cdf: 49 0f 44 c6 cmove %r14,%rax
> cmd->device->host->hostt->cmd_size;
> 2ce3: 48 8b 50 38 mov 0x38(%rax),%rdx
> 2ce7: 48 8b 12 mov (%rdx),%rdx <==============
> 2cea: 48 8b 92 98 00 00 00 mov 0x98(%rdx),%rdx
> 2cf1: 8b 92 30 01 00 00 mov 0x130(%rdx),%edx
> cmd->sdb.table.sgl = (void *)cmd + sizeof(struct scsi_cmnd) +
> 2cf7: 48 8d 94 10 b0 01 00 lea 0x1b0(%rax,%rdx,1),%rdx
> 2cfe: 00
> 2cff: 48 89 90 00 01 00 00 mov %rdx,0x100(%rax)
> blk_mq_start_request(req)
> .....
>
> It might not be your patchset, but the location does look suspicious.
Hi Doug,
I think this means that the scsi_in_cmd(cmd)->device pointer is NULL. I will
address this in the next version of this patch series.
Thanks,
Bart.