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.

Reply via email to