Re: [PATCH 2/3] scsi: Do not rely on blk-mq for double completions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-11-14 at 09:26 -0700, Keith Busch wrote:
+AD4 The scsi timeout error handling had been directly updating the request
+AD4 state to prevent a natural completion and error handling from completing
+AD4 the same request twice. Fix this layering violation by having scsi
+AD4 control the fate of its commands with scsi owned flags rather than
+AD4 use blk-mq's.
+AD4 
+AD4 Signed-off-by: Keith Busch +ADw-keith.busch+AEA-intel.com+AD4
+AD4 ---
+AD4  drivers/scsi/scsi+AF8-error.c +AHw 17 +-+-+---------------
+AD4  drivers/scsi/scsi+AF8-lib.c   +AHw  6 +-+-+-+-+--
+AD4  include/scsi/scsi+AF8-cmnd.h  +AHw  5 +-+-+-+--
+AD4  3 files changed, 12 insertions(+-), 16 deletions(-)
+AD4 
+AD4 diff --git a/drivers/scsi/scsi+AF8-error.c b/drivers/scsi/scsi+AF8-error.c
+AD4 index c736d61b1648..f89e829a1c51 100644
+AD4 --- a/drivers/scsi/scsi+AF8-error.c
+AD4 +-+-+- b/drivers/scsi/scsi+AF8-error.c
+AD4 +AEAAQA -199,6 +-199,9 +AEAAQA scsi+AF8-abort+AF8-command(struct scsi+AF8-cmnd +ACo-scmd)
+AD4  		return FAILED+ADs
+AD4  	+AH0
+AD4  
+AD4 +-	if (test+AF8-and+AF8-set+AF8-bit(+AF8AXw-SCMD+AF8-COMPLETE, +ACY-scmd-+AD4-flags))
+AD4 +-		return SUCCESS+ADs
+AD4 +-
+AD4  	spin+AF8-lock+AF8-irqsave(shost-+AD4-host+AF8-lock, flags)+ADs
+AD4  	if (shost-+AD4-eh+AF8-deadline +ACEAPQ -1 +ACYAJg +ACE-shost-+AD4-last+AF8-reset)
+AD4  		shost-+AD4-last+AF8-reset +AD0 jiffies+ADs
+AD4 +AEAAQA -296,20 +-299,6 +AEAAQA enum blk+AF8-eh+AF8-timer+AF8-return scsi+AF8-times+AF8-out(struct request
+AD4 +ACo-req)
+AD4  		rtn +AD0 host-+AD4-hostt-+AD4-eh+AF8-timed+AF8-out(scmd)+ADs
+AD4  
+AD4  	if (rtn +AD0APQ BLK+AF8-EH+AF8-DONE) +AHs
+AD4 -		/+ACo
+AD4 -		 +ACo For blk-mq, we must set the request state to complete
+AD4 now
+AD4 -		 +ACo before sending the request to the scsi error handler.
+AD4 This
+AD4 -		 +ACo will prevent a use-after-free in the event the LLD
+AD4 manages
+AD4 -		 +ACo to complete the request before the error handler
+AD4 finishes
+AD4 -		 +ACo processing this timed out request.
+AD4 -		 +ACo
+AD4 -		 +ACo If the request was already completed, then the LLD beat
+AD4 the
+AD4 -		 +ACo time out handler from transferring the request to the
+AD4 scsi
+AD4 -		 +ACo error handler. In that case we can return immediately
+AD4 as no
+AD4 -		 +ACo further action is required.
+AD4 -		 +ACo-/
+AD4 -		if (req-+AD4-q-+AD4-mq+AF8-ops +ACYAJg +ACE-blk+AF8-mq+AF8-mark+AF8-complete(req))
+AD4 -			return rtn+ADs
+AD4  		if (scsi+AF8-abort+AF8-command(scmd) +ACEAPQ SUCCESS) +AHs
+AD4  			set+AF8-host+AF8-byte(scmd, DID+AF8-TIME+AF8-OUT)+ADs
+AD4  			scsi+AF8-eh+AF8-scmd+AF8-add(scmd)+ADs
+AD4 diff --git a/drivers/scsi/scsi+AF8-lib.c b/drivers/scsi/scsi+AF8-lib.c
+AD4 index c7fccbb8f554..1e74137f1073 100644
+AD4 --- a/drivers/scsi/scsi+AF8-lib.c
+AD4 +-+-+- b/drivers/scsi/scsi+AF8-lib.c
+AD4 +AEAAQA -2044,8 +-2044,11 +AEAAQA static int scsi+AF8-mq+AF8-prep+AF8-fn(struct request +ACo-req)
+AD4  
+AD4  static void scsi+AF8-mq+AF8-done(struct scsi+AF8-cmnd +ACo-cmd)
+AD4  +AHs
+AD4 +-	if (test+AF8-and+AF8-set+AF8-bit(+AF8AXw-SCMD+AF8-COMPLETE, +ACY-cmd-+AD4-flags))
+AD4 +-		return+ADs
+AD4  	trace+AF8-scsi+AF8-dispatch+AF8-cmd+AF8-done(cmd)+ADs
+AD4 -	blk+AF8-mq+AF8-complete+AF8-request(cmd-+AD4-request)+ADs
+AD4 +-	if (unlikely(+ACE-blk+AF8-mq+AF8-complete+AF8-request(cmd-+AD4-request)))
+AD4 +-		clear+AF8-bit(+AF8AXw-SCMD+AF8-COMPLETE, +ACY-cmd-+AD4-flags)+ADs
+AD4  +AH0
+AD4  
+AD4  static void scsi+AF8-mq+AF8-put+AF8-budget(struct blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx)
+AD4 +AEAAQA -2104,6 +-2107,7 +AEAAQA static blk+AF8-status+AF8-t scsi+AF8-queue+AF8-rq(struct
+AD4 blk+AF8-mq+AF8-hw+AF8-ctx +ACo-hctx,
+AD4  			goto out+AF8-dec+AF8-host+AF8-busy+ADs
+AD4  		req-+AD4-rq+AF8-flags +AHwAPQ RQF+AF8-DONTPREP+ADs
+AD4  	+AH0 else +AHs
+AD4 +-		cmd-+AD4-flags +ACYAPQ +AH4-SCMD+AF8-COMPLETE+ADs
+AD4  		blk+AF8-mq+AF8-start+AF8-request(req)+ADs
+AD4  	+AH0


Hi Keith,

Please Cc Martin Petersen and the scsi mailing list for SCSI patches.

Regarding this patch: I think this patch introduces a subtle but severe bug
in the SCSI core, namely that if an abort is processed concurrently with
request completion with +ACI-fake timeout+ACI enabled that the abort is ignored.

Bart.



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux