> -----Original Message----- > From: Bart Van Assche [mailto:Bart.VanAssche@xxxxxxxxxxx] > Sent: Monday, May 15, 2017 4:03 PM > To: jejb@xxxxxxxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Long Li <longli@xxxxxxxxxxxxx>; > martin.petersen@xxxxxxxxxx > Cc: Long Li <longli@xxxxxxxxxxxxx> > Subject: Re: [Possible Phish Fraud][PATCH] scsi: zero per-cmd driver data for > each MQ I/O > > On Wed, 2017-05-10 at 14:07 -0700, Long Li wrote: > > From: Long Li <longli@xxxxxxxxxxxxx> > > > > Lower layer driver may not initialize private data before use. Zero > > them out to prevent use of stale data. > > > > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx> > > --- > > drivers/scsi/scsi_lib.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index > > 19125d7..a821593 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -1850,7 +1850,7 @@ static int scsi_mq_prep_fn(struct request *req) > > > > /* zero out the cmd, except for the embedded scsi_request */ > > memset((char *)cmd + sizeof(cmd->req), 0, > > - sizeof(*cmd) - sizeof(cmd->req)); > > + sizeof(*cmd) - sizeof(cmd->req) + shost->hostt->cmd_size); > > > > req->special = cmd; > > Hello Long, > > Sorry but this patch looks wrong to me. Since scsi_mq_prep_fn() is called > after scsi_req_init(), erasing struct scsi_request from scsi_mq_prep_fn() will > erase the values that were set by scsi_req_init(). That includes information > like the pointer to the SCSI CDB and the CDB itself. See e.g. > scsi_execute(). > > Did you come up with this patch after source reading or did you come up with > this patch while chasing a bug? Thanks for looking! Yes this is for chasing a bug. Actually scsi_mq_prep_fn() doesn't touch cmd->req (which is the struct scsi_request mentioned in your email). With the patch, we also zero the private data used by lower layer driver, in addition to the private data in scsi_cmnd. > > Thanks, > > Bart.
