Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [High] scsi-multipath: NULL dereference on bio-less request in scsi_mpath_end_request_no_update() --
commit 6015c0421ae4c77548304e8f223ec4e93492aa00 Author: John Garry <[email protected]> scsi-multipath: update request for terminal request queuing error This commit adds scsi_mpath_end_request_no_update() and calls it from scsi_queue_rq() when a multipath request fails with a terminal queueing error, updating active request counts and I/O statistics. > diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c > index 93a4545f1dbf7..bec6dd94f3be8 100644 > --- a/drivers/scsi/scsi_multipath.c > +++ b/drivers/scsi/scsi_multipath.c > @@ -614,6 +614,41 @@ bool scsi_mpath_end_request(struct request *req, > blk_status_t error, > return false; > } > > +void scsi_mpath_end_request_no_update(struct request *req, blk_status_t > error) > +{ > + struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req); > + struct scsi_device *sdev = scmd->device; > + struct bio *clone = req->bio, *master = clone->bi_private; > + struct block_device *bi_bdev = master->bi_bdev; > + unsigned int nr_bytes; > + > + /* Don't generate error logs if we are going to try another path */ > + if (blk_path_error(error)) > + req->rq_flags |= RQF_QUIET; > + > + if (scmd->flags & SCMD_MPATH_CNT_ACTIVE) { > + struct Scsi_Host *shost = sdev->host; > + > + atomic_dec_if_positive(&shost->mpath_nr_active); > + } > + > + if (!(scmd->flags & SCMD_MPATH_IO_STATS)) > + return; [Severity: High] Can clone->bi_private cause a NULL pointer dereference when req->bio is NULL? blk_kick_flush() initializes fq->flush_rq with flush_rq->bio set to NULL and copies REQ_DRV (which REQ_MPATH aliases) into flush_rq->cmd_flags: block/blk-flush.c:blk_kick_flush() { ... flush_rq->cmd_flags = REQ_OP_FLUSH | REQ_PREFLUSH; flush_rq->cmd_flags |= (flags & REQ_DRV) | (flags & REQ_FAILFAST_MASK); ... } Unlike scsi_mpath_start_request() and scsi_mpath_end_request(), which only dereference req->bio when SCMD_MPATH_IO_STATS is set, scsi_mpath_end_request_no_update() dereferences req->bio unconditionally at function entry before checking SCMD_MPATH_IO_STATS. If a flush request hits a terminal queueing error in scsi_queue_rq() (such as when a path is transport offline), this crashes. Deferring the req->bio and clone->bi_private dereferences until after the SCMD_MPATH_IO_STATS check would avoid this. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=22
