| Message ID | 20200124045014.23554-1-hmadhani@marvell.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 00fe717ee1ea3c2979db4f94b1533c57aed8dea9 |
| Headers | show |
| Series | [v5] qla2xxx: Fix unbound NVME response length | expand |
On Thu, 2020-01-23 at 20:50 -0800, Himanshu Madhani wrote: > From: Arun Easi <aeasi@marvell.com> > > On certain cases when response length is less than 32, NVME response data > is supplied inline in IOCB. This is indicated by some combination of state > flags. There was an instance when a high, and incorrect, response length was > indicated causing driver to overrun buffers. Fix this by checking and > limiting the response payload length. > > Fixes: 7401bc18d1ee3 ("scsi: qla2xxx: Add FC-NVMe command handling") > Cc: stable@vger.kernel.org > Signed-off-by: Arun Easi <aeasi@marvell.com> > Signed-off-by: Himanshu Madhani <hmadhani@marvell.com> > --- > Hi Martin, > > We discovered issue with our newer Gen7 adapter when response length > happens to be larger than 32 bytes, could result into crash. > > Please apply this to 5.5/scsi-fixes branch at your earliest convenience. > > Changes from v4 -> v5 > > o Added WARN_ONCE and moved it under ql_dbg bits to avoid excessive logging > > Changes from v3 -> v4 > > o use "sizeof(struct nvme_fc_ersp_iu)" in missed place. > > Changes from v2 -> v3 > > o Use "sizeof(struct nvme_fc_ersp_iu)" to indicate response payload size. > > Changes from v1 -> v2 > > o Fixed the tag for stable. > o Removed logit which got spilled from other patch to prevent compile failure. > > Thanks, > Himanshu > --- > drivers/scsi/qla2xxx/qla_dbg.c | 6 ------ > drivers/scsi/qla2xxx/qla_dbg.h | 6 ++++++ > drivers/scsi/qla2xxx/qla_isr.c | 12 ++++++++++++ > 3 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c > index e5500bba06ca..88a56e8480f7 100644 > --- a/drivers/scsi/qla2xxx/qla_dbg.c > +++ b/drivers/scsi/qla2xxx/qla_dbg.c > @@ -2519,12 +2519,6 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked) > /* Driver Debug Functions. */ > /****************************************************************************/ > > -static inline int > -ql_mask_match(uint level) > -{ > - return (level & ql2xextended_error_logging) == level; > -} > - > /* > * This function is for formatting and logging debug information. > * It is to be used when vha is available. It formats the message > diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h > index bb01b680ce9f..433e95502808 100644 > --- a/drivers/scsi/qla2xxx/qla_dbg.h > +++ b/drivers/scsi/qla2xxx/qla_dbg.h > @@ -374,3 +374,9 @@ extern int qla24xx_dump_ram(struct qla_hw_data *, uint32_t, uint32_t *, > extern void qla24xx_pause_risc(struct device_reg_24xx __iomem *, > struct qla_hw_data *); > extern int qla24xx_soft_reset(struct qla_hw_data *); > + > +static inline int > +ql_mask_match(uint level) > +{ > + return (level & ql2xextended_error_logging) == level; > +} > diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c > index e7bad0bfffda..e40705d38cea 100644 > --- a/drivers/scsi/qla2xxx/qla_isr.c > +++ b/drivers/scsi/qla2xxx/qla_isr.c > @@ -1939,6 +1939,18 @@ static void qla24xx_nvme_iocb_entry(scsi_qla_host_t *vha, struct req_que *req, > inbuf = (uint32_t *)&sts->nvme_ersp_data; > outbuf = (uint32_t *)fd->rspaddr; > iocb->u.nvme.rsp_pyld_len = le16_to_cpu(sts->nvme_rsp_pyld_len); > + if (unlikely(iocb->u.nvme.rsp_pyld_len > > + sizeof(struct nvme_fc_ersp_iu))) { > + if (ql_mask_match(ql_dbg_io)) { > + WARN_ONCE(1, "Unexpected response payload length %u.\n", > + iocb->u.nvme.rsp_pyld_len); > + ql_log(ql_log_warn, fcport->vha, 0x5100, > + "Unexpected response payload length %u.\n", > + iocb->u.nvme.rsp_pyld_len); > + } > + iocb->u.nvme.rsp_pyld_len = > + sizeof(struct nvme_fc_ersp_iu); > + } > iter = iocb->u.nvme.rsp_pyld_len >> 2; > for (; iter; iter--) > *outbuf++ = swab32(*inbuf++); v5 Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Himanshu, > On certain cases when response length is less than 32, NVME response > data is supplied inline in IOCB. This is indicated by some combination > of state flags. There was an instance when a high, and incorrect, > response length was indicated causing driver to overrun buffers. Fix > this by checking and limiting the response payload length. Applied to 5.6/scsi-fixes. Thanks!
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index e5500bba06ca..88a56e8480f7 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -2519,12 +2519,6 @@ qla83xx_fw_dump(scsi_qla_host_t *vha, int hardware_locked) /* Driver Debug Functions. */ /****************************************************************************/ -static inline int -ql_mask_match(uint level) -{ - return (level & ql2xextended_error_logging) == level; -} - /* * This function is for formatting and logging debug information. * It is to be used when vha is available. It formats the message diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h index bb01b680ce9f..433e95502808 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.h +++ b/drivers/scsi/qla2xxx/qla_dbg.h @@ -374,3 +374,9 @@ extern int qla24xx_dump_ram(struct qla_hw_data *, uint32_t, uint32_t *, extern void qla24xx_pause_risc(struct device_reg_24xx __iomem *, struct qla_hw_data *); extern int qla24xx_soft_reset(struct qla_hw_data *); + +static inline int +ql_mask_match(uint level) +{ + return (level & ql2xextended_error_logging) == level; +} diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index e7bad0bfffda..e40705d38cea 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -1939,6 +1939,18 @@ static void qla24xx_nvme_iocb_entry(scsi_qla_host_t *vha, struct req_que *req, inbuf = (uint32_t *)&sts->nvme_ersp_data; outbuf = (uint32_t *)fd->rspaddr; iocb->u.nvme.rsp_pyld_len = le16_to_cpu(sts->nvme_rsp_pyld_len); + if (unlikely(iocb->u.nvme.rsp_pyld_len > + sizeof(struct nvme_fc_ersp_iu))) { + if (ql_mask_match(ql_dbg_io)) { + WARN_ONCE(1, "Unexpected response payload length %u.\n", + iocb->u.nvme.rsp_pyld_len); + ql_log(ql_log_warn, fcport->vha, 0x5100, + "Unexpected response payload length %u.\n", + iocb->u.nvme.rsp_pyld_len); + } + iocb->u.nvme.rsp_pyld_len = + sizeof(struct nvme_fc_ersp_iu); + } iter = iocb->u.nvme.rsp_pyld_len >> 2; for (; iter; iter--) *outbuf++ = swab32(*inbuf++);