[PATCH v4 0/5] scsi: mpi3mr: fix issues found by KASAN

[PATCH v4 0/5] scsi: mpi3mr: fix issues found by KASAN

[Shin'ichiro Kawasaki]

While I downloaded new firmware to eHBA-9600 on KASAN enabled kernel, I observed
three BUGs. The first three patches resolve one of the BUGs and related two more
issues found during the review. The following two patches resolve the two left
BUGs respectively.

Changes from v3:
* Moved 1st patch to 3rd to resolve a compiler warning

Changes from v2:
* Added 1st and 2nd patches to address issues found during review
* 3rd patch: changed fix approach per comment on the list
* Added Acked-by and Reviewed-by tags

Changes from v1:
* 2nd patch: Modified to use bitmap helper functions and number of bits
* 1st/3rd patches: Reflected a comment on the list and added Reviewed-by tags

Shin'ichiro Kawasaki (5):
  scsi: mpi3mr: fix calculation of valid entry length in alltgt_info
  scsi: mpi3mr: fix alltgt_info copy size
  scsi: mpi3mr: remove unnecessary memcpy
  scsi: mpi3mr: use number of bits to manage bitmap sizes
  scsi: mpi3mr: fix missing mrioc->evtack_cmds initialization

 drivers/scsi/mpi3mr/mpi3mr.h     | 10 +----
 drivers/scsi/mpi3mr/mpi3mr_app.c | 18 +++------
 drivers/scsi/mpi3mr/mpi3mr_fw.c  | 68 +++++++++++++-------------------
 drivers/scsi/mpi3mr/mpi3mr_os.c  |  4 ++
 4 files changed, 39 insertions(+), 61 deletions(-)

-- 
2.38.1

[PATCH v4 1/5] scsi: mpi3mr: fix calculation of valid entry length in alltgt_info

[Shin'ichiro Kawasaki]

The function mpi3mr_get_all_tgt_info calculates valid entry length in
alltgt_info whose type is pointer to struct mpi3mr_device_map_info.
However, the calculation assumes that the struct would have size of u32.
This results in wrong entry length. Fix the calculation to use the size
of *alltgt_info in place of u32.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Cc: stable@vger.kernel.org
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 9baac224b213..49916ae617e5 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -346,7 +346,8 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 
 	memcpy(&alltgt_info->num_devices, &num_devices, sizeof(num_devices));
 
-	usr_entrylen = (job->request_payload.payload_len - sizeof(u32)) / sizeof(*devmap_info);
+	usr_entrylen = (job->request_payload.payload_len - sizeof(*alltgt_info))
+		/ sizeof(*devmap_info);
 	usr_entrylen *= sizeof(*devmap_info);
 	min_entrylen = min(usr_entrylen, kern_entrylen);
 	if (min_entrylen && (!memcpy(&alltgt_info->dmi, devmap_info, min_entrylen))) {
-- 
2.38.1

[PATCH v4 2/5] scsi: mpi3mr: fix alltgt_info copy size

[Shin'ichiro Kawasaki]

The function mpi3mr_get_all_tgt_info calculates min_entrylen which holds
the valid entry length in alltgt_info. However, it does not refer
min_entrylen when it calls sg_copy_from_buffer to copy the valid entries
from alltgt_info to job->request_payload. Instead, it specifies the
payload length which is larger than the alltgt_info size, then it causes
"BUG: KASAN: slab-out-of-bounds". Fix the BUG by specifying the correct
length referring the calculated min_entrylen.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Cc: stable@vger.kernel.org
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 49916ae617e5..7fb9505723cf 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -359,7 +359,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 
 	sg_copy_from_buffer(job->request_payload.sg_list,
 			    job->request_payload.sg_cnt,
-			    alltgt_info, job->request_payload.payload_len);
+			    alltgt_info, sizeof(*alltgt_info) + min_entrylen);
 	rval = 0;
 out:
 	kfree(alltgt_info);
-- 
2.38.1

[PATCH v4 3/5] scsi: mpi3mr: remove unnecessary memcpy

[Shin'ichiro Kawasaki]

In the function mpi3mr_get_all_tgt_info, devmap_info points to
alltgt_info->dmi then there is no need to memcpy data from devmap_info
to alltgt_info->dmi. Remove the unnecessary memcpy. This also allows to
remove the local variable 'rval' and the goto label 'out'.

Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
Cc: stable@vger.kernel.org
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/scsi/mpi3mr/mpi3mr_app.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
index 7fb9505723cf..3b4ae044f4c0 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_app.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
@@ -293,7 +293,6 @@ static long mpi3mr_bsg_pel_enable(struct mpi3mr_ioc *mrioc,
 static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 	struct bsg_job *job)
 {
-	long rval = -EINVAL;
 	u16 num_devices = 0, i = 0, size;
 	unsigned long flags;
 	struct mpi3mr_tgt_dev *tgtdev;
@@ -304,7 +303,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 	if (job->request_payload.payload_len < sizeof(u32)) {
 		dprint_bsg_err(mrioc, "%s: invalid size argument\n",
 		    __func__);
-		return rval;
+		return -EINVAL;
 	}
 
 	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
@@ -350,20 +349,12 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
 		/ sizeof(*devmap_info);
 	usr_entrylen *= sizeof(*devmap_info);
 	min_entrylen = min(usr_entrylen, kern_entrylen);
-	if (min_entrylen && (!memcpy(&alltgt_info->dmi, devmap_info, min_entrylen))) {
-		dprint_bsg_err(mrioc, "%s:%d: device map info copy failed\n",
-		    __func__, __LINE__);
-		rval = -EFAULT;
-		goto out;
-	}
 
 	sg_copy_from_buffer(job->request_payload.sg_list,
 			    job->request_payload.sg_cnt,
 			    alltgt_info, sizeof(*alltgt_info) + min_entrylen);
-	rval = 0;
-out:
 	kfree(alltgt_info);
-	return rval;
+	return 0;
 }
 /**
  * mpi3mr_get_change_count - Get topology change count
-- 
2.38.1

[PATCH v4 4/5] scsi: mpi3mr: use number of bits to manage bitmap sizes

[Shin'ichiro Kawasaki]

To allocate bitmaps, the mpi3mr driver calculates sizes of bitmaps using
byte as unit. However, bitmap helper functions assume that bitmaps are
allocated using unsigned long as unit. This gap causes memory access
beyond the bitmap sizes and results in "BUG: KASAN: slab-out-of-bounds".
The BUG was observed at firmware download to eHBA-9600. Call trace
indicated that the out-of-bounds access happened in find_first_zero_bit
called from mpi3mr_send_event_ack for miroc->evtack_cmds_bitmap.

To fix the BUG, do not use bytes to manage bitmap sizes. Instead, use
number of bits, and call bitmap helper functions which take number of
bits as arguments. For memory allocation, call bitmap_zalloc instead of
kzalloc. For zero clear, call bitmap_clear instead of memset. For
resize, call bitmap_zalloc and bitmap_copy instead of krealloc.

Remove three fields for bitmap byte sizes in struct scmd_priv, which are
no longer required. Replace the field dev_handle_bitmap_sz with
dev_handle_bitmap_bits to keep number of bits of removepend_bitmap
across resize.

Fixes: c5758fc72b92 ("scsi: mpi3mr: Gracefully handle online FW update operation")
Fixes: e844adb1fbdc ("scsi: mpi3mr: Implement SCSI error handler hooks")
Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic")
Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/scsi/mpi3mr/mpi3mr.h    | 10 +----
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 68 ++++++++++++++-------------------
 2 files changed, 30 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index def4c5e15cd8..8a438f248a82 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -955,19 +955,16 @@ struct scmd_priv {
  * @chain_buf_count: Chain buffer count
  * @chain_buf_pool: Chain buffer pool
  * @chain_sgl_list: Chain SGL list
- * @chain_bitmap_sz: Chain buffer allocator bitmap size
  * @chain_bitmap: Chain buffer allocator bitmap
  * @chain_buf_lock: Chain buffer list lock
  * @bsg_cmds: Command tracker for BSG command
  * @host_tm_cmds: Command tracker for task management commands
  * @dev_rmhs_cmds: Command tracker for device removal commands
  * @evtack_cmds: Command tracker for event ack commands
- * @devrem_bitmap_sz: Device removal bitmap size
  * @devrem_bitmap: Device removal bitmap
- * @dev_handle_bitmap_sz: Device handle bitmap size
+ * @dev_handle_bitmap_bits: Number of bits in device handle bitmap
  * @removepend_bitmap: Remove pending bitmap
  * @delayed_rmhs_list: Delayed device removal list
- * @evtack_cmds_bitmap_sz: Event Ack bitmap size
  * @evtack_cmds_bitmap: Event Ack bitmap
  * @delayed_evtack_cmds_list: Delayed event acknowledgment list
  * @ts_update_counter: Timestamp update counter
@@ -1128,7 +1125,6 @@ struct mpi3mr_ioc {
 	u32 chain_buf_count;
 	struct dma_pool *chain_buf_pool;
 	struct chain_element *chain_sgl_list;
-	u16  chain_bitmap_sz;
 	void *chain_bitmap;
 	spinlock_t chain_buf_lock;
 
@@ -1136,12 +1132,10 @@ struct mpi3mr_ioc {
 	struct mpi3mr_drv_cmd host_tm_cmds;
 	struct mpi3mr_drv_cmd dev_rmhs_cmds[MPI3MR_NUM_DEVRMCMD];
 	struct mpi3mr_drv_cmd evtack_cmds[MPI3MR_NUM_EVTACKCMD];
-	u16 devrem_bitmap_sz;
 	void *devrem_bitmap;
-	u16 dev_handle_bitmap_sz;
+	u16 dev_handle_bitmap_bits;
 	void *removepend_bitmap;
 	struct list_head delayed_rmhs_list;
-	u16 evtack_cmds_bitmap_sz;
 	void *evtack_cmds_bitmap;
 	struct list_head delayed_evtack_cmds_list;
 
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 286a44506578..d25cd0382e20 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -1128,7 +1128,6 @@ static int mpi3mr_issue_and_process_mur(struct mpi3mr_ioc *mrioc,
 static int
 mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
 {
-	u16 dev_handle_bitmap_sz;
 	void *removepend_bitmap;
 
 	if (mrioc->facts.reply_sz > mrioc->reply_sz) {
@@ -1160,25 +1159,24 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
 		    "\tcontroller while sas transport support is enabled at the\n"
 		    "\tdriver, please reboot the system or reload the driver\n");
 
-	dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
-	if (mrioc->facts.max_devhandle % 8)
-		dev_handle_bitmap_sz++;
-	if (dev_handle_bitmap_sz > mrioc->dev_handle_bitmap_sz) {
-		removepend_bitmap = krealloc(mrioc->removepend_bitmap,
-		    dev_handle_bitmap_sz, GFP_KERNEL);
+	if (mrioc->facts.max_devhandle > mrioc->dev_handle_bitmap_bits) {
+		removepend_bitmap = bitmap_zalloc(mrioc->facts.max_devhandle,
+						  GFP_KERNEL);
 		if (!removepend_bitmap) {
 			ioc_err(mrioc,
-			    "failed to increase removepend_bitmap sz from: %d to %d\n",
-			    mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
+				"failed to increase removepend_bitmap bits from %d to %d\n",
+				mrioc->dev_handle_bitmap_bits,
+				mrioc->facts.max_devhandle);
 			return -EPERM;
 		}
-		memset(removepend_bitmap + mrioc->dev_handle_bitmap_sz, 0,
-		    dev_handle_bitmap_sz - mrioc->dev_handle_bitmap_sz);
+		bitmap_copy(removepend_bitmap, mrioc->removepend_bitmap,
+			    mrioc->dev_handle_bitmap_bits);
 		mrioc->removepend_bitmap = removepend_bitmap;
 		ioc_info(mrioc,
-		    "increased dev_handle_bitmap_sz from %d to %d\n",
-		    mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
-		mrioc->dev_handle_bitmap_sz = dev_handle_bitmap_sz;
+			 "increased bits of dev_handle_bitmap from %d to %d\n",
+			 mrioc->dev_handle_bitmap_bits,
+			 mrioc->facts.max_devhandle);
+		mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
 	}
 
 	return 0;
@@ -2957,27 +2955,18 @@ static int mpi3mr_alloc_reply_sense_bufs(struct mpi3mr_ioc *mrioc)
 	if (!mrioc->pel_abort_cmd.reply)
 		goto out_failed;
 
-	mrioc->dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
-	if (mrioc->facts.max_devhandle % 8)
-		mrioc->dev_handle_bitmap_sz++;
-	mrioc->removepend_bitmap = kzalloc(mrioc->dev_handle_bitmap_sz,
-	    GFP_KERNEL);
+	mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
+	mrioc->removepend_bitmap = bitmap_zalloc(mrioc->dev_handle_bitmap_bits,
+						 GFP_KERNEL);
 	if (!mrioc->removepend_bitmap)
 		goto out_failed;
 
-	mrioc->devrem_bitmap_sz = MPI3MR_NUM_DEVRMCMD / 8;
-	if (MPI3MR_NUM_DEVRMCMD % 8)
-		mrioc->devrem_bitmap_sz++;
-	mrioc->devrem_bitmap = kzalloc(mrioc->devrem_bitmap_sz,
-	    GFP_KERNEL);
+	mrioc->devrem_bitmap = bitmap_zalloc(MPI3MR_NUM_DEVRMCMD, GFP_KERNEL);
 	if (!mrioc->devrem_bitmap)
 		goto out_failed;
 
-	mrioc->evtack_cmds_bitmap_sz = MPI3MR_NUM_EVTACKCMD / 8;
-	if (MPI3MR_NUM_EVTACKCMD % 8)
-		mrioc->evtack_cmds_bitmap_sz++;
-	mrioc->evtack_cmds_bitmap = kzalloc(mrioc->evtack_cmds_bitmap_sz,
-	    GFP_KERNEL);
+	mrioc->evtack_cmds_bitmap = bitmap_zalloc(MPI3MR_NUM_EVTACKCMD,
+						  GFP_KERNEL);
 	if (!mrioc->evtack_cmds_bitmap)
 		goto out_failed;
 
@@ -3415,10 +3404,7 @@ static int mpi3mr_alloc_chain_bufs(struct mpi3mr_ioc *mrioc)
 		if (!mrioc->chain_sgl_list[i].addr)
 			goto out_failed;
 	}
-	mrioc->chain_bitmap_sz = num_chains / 8;
-	if (num_chains % 8)
-		mrioc->chain_bitmap_sz++;
-	mrioc->chain_bitmap = kzalloc(mrioc->chain_bitmap_sz, GFP_KERNEL);
+	mrioc->chain_bitmap = bitmap_zalloc(num_chains, GFP_KERNEL);
 	if (!mrioc->chain_bitmap)
 		goto out_failed;
 	return retval;
@@ -4189,10 +4175,11 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc)
 		for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++)
 			memset(mrioc->evtack_cmds[i].reply, 0,
 			    sizeof(*mrioc->evtack_cmds[i].reply));
-		memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
-		memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
-		memset(mrioc->evtack_cmds_bitmap, 0,
-		    mrioc->evtack_cmds_bitmap_sz);
+		bitmap_clear(mrioc->removepend_bitmap, 0,
+			     mrioc->dev_handle_bitmap_bits);
+		bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
+		bitmap_clear(mrioc->evtack_cmds_bitmap, 0,
+			     MPI3MR_NUM_EVTACKCMD);
 	}
 
 	for (i = 0; i < mrioc->num_queues; i++) {
@@ -4886,9 +4873,10 @@ int mpi3mr_soft_reset_handler(struct mpi3mr_ioc *mrioc,
 
 	mpi3mr_flush_delayed_cmd_lists(mrioc);
 	mpi3mr_flush_drv_cmds(mrioc);
-	memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
-	memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
-	memset(mrioc->evtack_cmds_bitmap, 0, mrioc->evtack_cmds_bitmap_sz);
+	bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
+	bitmap_clear(mrioc->removepend_bitmap, 0,
+		     mrioc->dev_handle_bitmap_bits);
+	bitmap_clear(mrioc->evtack_cmds_bitmap, 0, MPI3MR_NUM_EVTACKCMD);
 	mpi3mr_flush_host_io(mrioc);
 	mpi3mr_cleanup_fwevt_list(mrioc);
 	mpi3mr_invalidate_devhandles(mrioc);
-- 
2.38.1

[PATCH v4 5/5] scsi: mpi3mr: fix missing mrioc->evtack_cmds initialization

[Shin'ichiro Kawasaki]

The commit c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic")
introduced an array mrioc->evtack_cmds. But initialization of the array
elements was missed. They are just zero cleared. The function
mpi3mr_complete_evt_ack refers host_tag field of the elements. Due to
zero value of the host_tag field, the functions calls clear_bit for
mrico->evtack_cmds_bitmap with wrong bit index. This results in memory
access to invalid address and "BUG: KASAN: use-after-free". This BUG was
observed at eHBA-9600 firmware update to version 8.3.1.0. To fix it, add
the missing initialization of mrioc->evtack_cmds.

Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic")
Cc: stable@vger.kernel.org
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/scsi/mpi3mr/mpi3mr_os.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 3306de7170f6..6eaeba41072c 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -4952,6 +4952,10 @@ mpi3mr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		mpi3mr_init_drv_cmd(&mrioc->dev_rmhs_cmds[i],
 		    MPI3MR_HOSTTAG_DEVRMCMD_MIN + i);
 
+	for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++)
+		mpi3mr_init_drv_cmd(&mrioc->evtack_cmds[i],
+				    MPI3MR_HOSTTAG_EVTACKCMD_MIN + i);
+
 	if (pdev->revision)
 		mrioc->enable_segqueue = true;
 
-- 
2.38.1

Re: [PATCH v4 0/5] scsi: mpi3mr: fix issues found by KASAN

[Martin K. Petersen]

> While I downloaded new firmware to eHBA-9600 on KASAN enabled kernel,
> I observed three BUGs. The first three patches resolve one of the BUGs
> and related two more issues found during the review. The following two
> patches resolve the two left BUGs respectively.

Broadcom: Please review and test. Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v4 2/5] scsi: mpi3mr: fix alltgt_info copy size

[Sathya Prakash Veerichetty]

[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]

On Thu, Jan 26, 2023 at 11:35 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> The function mpi3mr_get_all_tgt_info calculates min_entrylen which holds
> the valid entry length in alltgt_info. However, it does not refer
> min_entrylen when it calls sg_copy_from_buffer to copy the valid entries
> from alltgt_info to job->request_payload. Instead, it specifies the
> payload length which is larger than the alltgt_info size, then it causes
> "BUG: KASAN: slab-out-of-bounds". Fix the BUG by specifying the correct
> length referring the calculated min_entrylen.

>>both this and the first patch could have been merged. We will do some more cleanup on this function and provide a new patch, we can hold 1 and 2 for now.
>
> Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  drivers/scsi/mpi3mr/mpi3mr_app.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c
> index 49916ae617e5..7fb9505723cf 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_app.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c
> @@ -359,7 +359,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc,
>
>         sg_copy_from_buffer(job->request_payload.sg_list,
>                             job->request_payload.sg_cnt,
> -                           alltgt_info, job->request_payload.payload_len);
> +                           alltgt_info, sizeof(*alltgt_info) + min_entrylen);
>         rval = 0;
>  out:
>         kfree(alltgt_info);
> --
> 2.38.1
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4227 bytes --]

Re: [PATCH v4 4/5] scsi: mpi3mr: use number of bits to manage bitmap sizes

[Sathya Prakash Veerichetty]

[-- Attachment #1: Type: text/plain, Size: 11674 bytes --]

On Thu, Jan 26, 2023 at 11:35 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> To allocate bitmaps, the mpi3mr driver calculates sizes of bitmaps using
> byte as unit. However, bitmap helper functions assume that bitmaps are
> allocated using unsigned long as unit. This gap causes memory access
> beyond the bitmap sizes and results in "BUG: KASAN: slab-out-of-bounds".
> The BUG was observed at firmware download to eHBA-9600. Call trace
> indicated that the out-of-bounds access happened in find_first_zero_bit
> called from mpi3mr_send_event_ack for miroc->evtack_cmds_bitmap.
>
> To fix the BUG, do not use bytes to manage bitmap sizes. Instead, use
> number of bits, and call bitmap helper functions which take number of
> bits as arguments. For memory allocation, call bitmap_zalloc instead of
> kzalloc. For zero clear, call bitmap_clear instead of memset. For
> resize, call bitmap_zalloc and bitmap_copy instead of krealloc.
>
> Remove three fields for bitmap byte sizes in struct scmd_priv, which are
> no longer required. Replace the field dev_handle_bitmap_sz with
> dev_handle_bitmap_bits to keep number of bits of removepend_bitmap
> across resize.
>
>>Thanks for getting this changed, can you please change the kfree for the bitmaps to bitmap_free for consistency of the API.
> Fixes: c5758fc72b92 ("scsi: mpi3mr: Gracefully handle online FW update operation")
> Fixes: e844adb1fbdc ("scsi: mpi3mr: Implement SCSI error handler hooks")
> Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic")
> Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  drivers/scsi/mpi3mr/mpi3mr.h    | 10 +----
>  drivers/scsi/mpi3mr/mpi3mr_fw.c | 68 ++++++++++++++-------------------
>  2 files changed, 30 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
> index def4c5e15cd8..8a438f248a82 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr.h
> +++ b/drivers/scsi/mpi3mr/mpi3mr.h
> @@ -955,19 +955,16 @@ struct scmd_priv {
>   * @chain_buf_count: Chain buffer count
>   * @chain_buf_pool: Chain buffer pool
>   * @chain_sgl_list: Chain SGL list
> - * @chain_bitmap_sz: Chain buffer allocator bitmap size
>   * @chain_bitmap: Chain buffer allocator bitmap
>   * @chain_buf_lock: Chain buffer list lock
>   * @bsg_cmds: Command tracker for BSG command
>   * @host_tm_cmds: Command tracker for task management commands
>   * @dev_rmhs_cmds: Command tracker for device removal commands
>   * @evtack_cmds: Command tracker for event ack commands
> - * @devrem_bitmap_sz: Device removal bitmap size
>   * @devrem_bitmap: Device removal bitmap
> - * @dev_handle_bitmap_sz: Device handle bitmap size
> + * @dev_handle_bitmap_bits: Number of bits in device handle bitmap
>   * @removepend_bitmap: Remove pending bitmap
>   * @delayed_rmhs_list: Delayed device removal list
> - * @evtack_cmds_bitmap_sz: Event Ack bitmap size
>   * @evtack_cmds_bitmap: Event Ack bitmap
>   * @delayed_evtack_cmds_list: Delayed event acknowledgment list
>   * @ts_update_counter: Timestamp update counter
> @@ -1128,7 +1125,6 @@ struct mpi3mr_ioc {
>         u32 chain_buf_count;
>         struct dma_pool *chain_buf_pool;
>         struct chain_element *chain_sgl_list;
> -       u16  chain_bitmap_sz;
>         void *chain_bitmap;
>         spinlock_t chain_buf_lock;
>
> @@ -1136,12 +1132,10 @@ struct mpi3mr_ioc {
>         struct mpi3mr_drv_cmd host_tm_cmds;
>         struct mpi3mr_drv_cmd dev_rmhs_cmds[MPI3MR_NUM_DEVRMCMD];
>         struct mpi3mr_drv_cmd evtack_cmds[MPI3MR_NUM_EVTACKCMD];
> -       u16 devrem_bitmap_sz;
>         void *devrem_bitmap;
> -       u16 dev_handle_bitmap_sz;
> +       u16 dev_handle_bitmap_bits;
>         void *removepend_bitmap;
>         struct list_head delayed_rmhs_list;
> -       u16 evtack_cmds_bitmap_sz;
>         void *evtack_cmds_bitmap;
>         struct list_head delayed_evtack_cmds_list;
>
> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index 286a44506578..d25cd0382e20 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> @@ -1128,7 +1128,6 @@ static int mpi3mr_issue_and_process_mur(struct mpi3mr_ioc *mrioc,
>  static int
>  mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
>  {
> -       u16 dev_handle_bitmap_sz;
>         void *removepend_bitmap;
>
>         if (mrioc->facts.reply_sz > mrioc->reply_sz) {
> @@ -1160,25 +1159,24 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
>                     "\tcontroller while sas transport support is enabled at the\n"
>                     "\tdriver, please reboot the system or reload the driver\n");
>
> -       dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
> -       if (mrioc->facts.max_devhandle % 8)
> -               dev_handle_bitmap_sz++;
> -       if (dev_handle_bitmap_sz > mrioc->dev_handle_bitmap_sz) {
> -               removepend_bitmap = krealloc(mrioc->removepend_bitmap,
> -                   dev_handle_bitmap_sz, GFP_KERNEL);
> +       if (mrioc->facts.max_devhandle > mrioc->dev_handle_bitmap_bits) {
>>Free the existing removepend_bitmap prior the alloc.
> +               removepend_bitmap = bitmap_zalloc(mrioc->facts.max_devhandle,
> +                                                 GFP_KERNEL);
>                 if (!removepend_bitmap) {
>                         ioc_err(mrioc,
> -                           "failed to increase removepend_bitmap sz from: %d to %d\n",
> -                           mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
> +                               "failed to increase removepend_bitmap bits from %d to %d\n",
> +                               mrioc->dev_handle_bitmap_bits,
> +                               mrioc->facts.max_devhandle);
>                         return -EPERM;
>                 }
> -               memset(removepend_bitmap + mrioc->dev_handle_bitmap_sz, 0,
> -                   dev_handle_bitmap_sz - mrioc->dev_handle_bitmap_sz);
> +               bitmap_copy(removepend_bitmap, mrioc->removepend_bitmap,
> +                           mrioc->dev_handle_bitmap_bits);
>>This copy is not needed as the data in the removepend_bitmap is not valid after reset and the zalloc already cleared the memory.

>                 mrioc->removepend_bitmap = removepend_bitmap;
>                 ioc_info(mrioc,
> -                   "increased dev_handle_bitmap_sz from %d to %d\n",
> -                   mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
> -               mrioc->dev_handle_bitmap_sz = dev_handle_bitmap_sz;
> +                        "increased bits of dev_handle_bitmap from %d to %d\n",
> +                        mrioc->dev_handle_bitmap_bits,
> +                        mrioc->facts.max_devhandle);
> +               mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
>         }
>
>         return 0;
> @@ -2957,27 +2955,18 @@ static int mpi3mr_alloc_reply_sense_bufs(struct mpi3mr_ioc *mrioc)
>         if (!mrioc->pel_abort_cmd.reply)
>                 goto out_failed;
>
> -       mrioc->dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
> -       if (mrioc->facts.max_devhandle % 8)
> -               mrioc->dev_handle_bitmap_sz++;
> -       mrioc->removepend_bitmap = kzalloc(mrioc->dev_handle_bitmap_sz,
> -           GFP_KERNEL);
> +       mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
> +       mrioc->removepend_bitmap = bitmap_zalloc(mrioc->dev_handle_bitmap_bits,
> +                                                GFP_KERNEL);
>         if (!mrioc->removepend_bitmap)
>                 goto out_failed;
>
> -       mrioc->devrem_bitmap_sz = MPI3MR_NUM_DEVRMCMD / 8;
> -       if (MPI3MR_NUM_DEVRMCMD % 8)
> -               mrioc->devrem_bitmap_sz++;
> -       mrioc->devrem_bitmap = kzalloc(mrioc->devrem_bitmap_sz,
> -           GFP_KERNEL);
> +       mrioc->devrem_bitmap = bitmap_zalloc(MPI3MR_NUM_DEVRMCMD, GFP_KERNEL);
>         if (!mrioc->devrem_bitmap)
>                 goto out_failed;
>
> -       mrioc->evtack_cmds_bitmap_sz = MPI3MR_NUM_EVTACKCMD / 8;
> -       if (MPI3MR_NUM_EVTACKCMD % 8)
> -               mrioc->evtack_cmds_bitmap_sz++;
> -       mrioc->evtack_cmds_bitmap = kzalloc(mrioc->evtack_cmds_bitmap_sz,
> -           GFP_KERNEL);
> +       mrioc->evtack_cmds_bitmap = bitmap_zalloc(MPI3MR_NUM_EVTACKCMD,
> +                                                 GFP_KERNEL);
>         if (!mrioc->evtack_cmds_bitmap)
>                 goto out_failed;
>
> @@ -3415,10 +3404,7 @@ static int mpi3mr_alloc_chain_bufs(struct mpi3mr_ioc *mrioc)
>                 if (!mrioc->chain_sgl_list[i].addr)
>                         goto out_failed;
>         }
> -       mrioc->chain_bitmap_sz = num_chains / 8;
> -       if (num_chains % 8)
> -               mrioc->chain_bitmap_sz++;
> -       mrioc->chain_bitmap = kzalloc(mrioc->chain_bitmap_sz, GFP_KERNEL);
> +       mrioc->chain_bitmap = bitmap_zalloc(num_chains, GFP_KERNEL);
>         if (!mrioc->chain_bitmap)
>                 goto out_failed;
>         return retval;
> @@ -4189,10 +4175,11 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc)
>                 for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++)
>                         memset(mrioc->evtack_cmds[i].reply, 0,
>                             sizeof(*mrioc->evtack_cmds[i].reply));
> -               memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
> -               memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
> -               memset(mrioc->evtack_cmds_bitmap, 0,
> -                   mrioc->evtack_cmds_bitmap_sz);
> +               bitmap_clear(mrioc->removepend_bitmap, 0,
> +                            mrioc->dev_handle_bitmap_bits);
> +               bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
> +               bitmap_clear(mrioc->evtack_cmds_bitmap, 0,
> +                            MPI3MR_NUM_EVTACKCMD);
>         }
>
>         for (i = 0; i < mrioc->num_queues; i++) {
> @@ -4886,9 +4873,10 @@ int mpi3mr_soft_reset_handler(struct mpi3mr_ioc *mrioc,
>
>         mpi3mr_flush_delayed_cmd_lists(mrioc);
>         mpi3mr_flush_drv_cmds(mrioc);
> -       memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
> -       memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
> -       memset(mrioc->evtack_cmds_bitmap, 0, mrioc->evtack_cmds_bitmap_sz);
> +       bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
> +       bitmap_clear(mrioc->removepend_bitmap, 0,
> +                    mrioc->dev_handle_bitmap_bits);
> +       bitmap_clear(mrioc->evtack_cmds_bitmap, 0, MPI3MR_NUM_EVTACKCMD);
>         mpi3mr_flush_host_io(mrioc);
>         mpi3mr_cleanup_fwevt_list(mrioc);
>         mpi3mr_invalidate_devhandles(mrioc);
> --
> 2.38.1
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4227 bytes --]

Re: [PATCH v4 4/5] scsi: mpi3mr: use number of bits to manage bitmap sizes

[Shinichiro Kawasaki]

On Feb 09, 2023 / 12:50, Sathya Prakash Veerichetty wrote:
> On Thu, Jan 26, 2023 at 11:35 PM Shin'ichiro Kawasaki
> <shinichiro.kawasaki@wdc.com> wrote:
> >
> > To allocate bitmaps, the mpi3mr driver calculates sizes of bitmaps using
> > byte as unit. However, bitmap helper functions assume that bitmaps are
> > allocated using unsigned long as unit. This gap causes memory access
> > beyond the bitmap sizes and results in "BUG: KASAN: slab-out-of-bounds".
> > The BUG was observed at firmware download to eHBA-9600. Call trace
> > indicated that the out-of-bounds access happened in find_first_zero_bit
> > called from mpi3mr_send_event_ack for miroc->evtack_cmds_bitmap.
> >
> > To fix the BUG, do not use bytes to manage bitmap sizes. Instead, use
> > number of bits, and call bitmap helper functions which take number of
> > bits as arguments. For memory allocation, call bitmap_zalloc instead of
> > kzalloc. For zero clear, call bitmap_clear instead of memset. For
> > resize, call bitmap_zalloc and bitmap_copy instead of krealloc.
> >
> > Remove three fields for bitmap byte sizes in struct scmd_priv, which are
> > no longer required. Replace the field dev_handle_bitmap_sz with
> > dev_handle_bitmap_bits to keep number of bits of removepend_bitmap
> > across resize.
> >
> >>Thanks for getting this changed, can you please change the kfree for the bitmaps to bitmap_free for consistency of the API.
> > Fixes: c5758fc72b92 ("scsi: mpi3mr: Gracefully handle online FW update operation")
> > Fixes: e844adb1fbdc ("scsi: mpi3mr: Implement SCSI error handler hooks")
> > Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic")
> > Fixes: 824a156633df ("scsi: mpi3mr: Base driver code")
> > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > ---
> >  drivers/scsi/mpi3mr/mpi3mr.h    | 10 +----
> >  drivers/scsi/mpi3mr/mpi3mr_fw.c | 68 ++++++++++++++-------------------
> >  2 files changed, 30 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
> > index def4c5e15cd8..8a438f248a82 100644
> > --- a/drivers/scsi/mpi3mr/mpi3mr.h
> > +++ b/drivers/scsi/mpi3mr/mpi3mr.h
> > @@ -955,19 +955,16 @@ struct scmd_priv {
> >   * @chain_buf_count: Chain buffer count
> >   * @chain_buf_pool: Chain buffer pool
> >   * @chain_sgl_list: Chain SGL list
> > - * @chain_bitmap_sz: Chain buffer allocator bitmap size
> >   * @chain_bitmap: Chain buffer allocator bitmap
> >   * @chain_buf_lock: Chain buffer list lock
> >   * @bsg_cmds: Command tracker for BSG command
> >   * @host_tm_cmds: Command tracker for task management commands
> >   * @dev_rmhs_cmds: Command tracker for device removal commands
> >   * @evtack_cmds: Command tracker for event ack commands
> > - * @devrem_bitmap_sz: Device removal bitmap size
> >   * @devrem_bitmap: Device removal bitmap
> > - * @dev_handle_bitmap_sz: Device handle bitmap size
> > + * @dev_handle_bitmap_bits: Number of bits in device handle bitmap
> >   * @removepend_bitmap: Remove pending bitmap
> >   * @delayed_rmhs_list: Delayed device removal list
> > - * @evtack_cmds_bitmap_sz: Event Ack bitmap size
> >   * @evtack_cmds_bitmap: Event Ack bitmap
> >   * @delayed_evtack_cmds_list: Delayed event acknowledgment list
> >   * @ts_update_counter: Timestamp update counter
> > @@ -1128,7 +1125,6 @@ struct mpi3mr_ioc {
> >         u32 chain_buf_count;
> >         struct dma_pool *chain_buf_pool;
> >         struct chain_element *chain_sgl_list;
> > -       u16  chain_bitmap_sz;
> >         void *chain_bitmap;
> >         spinlock_t chain_buf_lock;
> >
> > @@ -1136,12 +1132,10 @@ struct mpi3mr_ioc {
> >         struct mpi3mr_drv_cmd host_tm_cmds;
> >         struct mpi3mr_drv_cmd dev_rmhs_cmds[MPI3MR_NUM_DEVRMCMD];
> >         struct mpi3mr_drv_cmd evtack_cmds[MPI3MR_NUM_EVTACKCMD];
> > -       u16 devrem_bitmap_sz;
> >         void *devrem_bitmap;
> > -       u16 dev_handle_bitmap_sz;
> > +       u16 dev_handle_bitmap_bits;
> >         void *removepend_bitmap;
> >         struct list_head delayed_rmhs_list;
> > -       u16 evtack_cmds_bitmap_sz;
> >         void *evtack_cmds_bitmap;
> >         struct list_head delayed_evtack_cmds_list;
> >
> > diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > index 286a44506578..d25cd0382e20 100644
> > --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > @@ -1128,7 +1128,6 @@ static int mpi3mr_issue_and_process_mur(struct mpi3mr_ioc *mrioc,
> >  static int
> >  mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
> >  {
> > -       u16 dev_handle_bitmap_sz;
> >         void *removepend_bitmap;
> >
> >         if (mrioc->facts.reply_sz > mrioc->reply_sz) {
> > @@ -1160,25 +1159,24 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
> >                     "\tcontroller while sas transport support is enabled at the\n"
> >                     "\tdriver, please reboot the system or reload the driver\n");
> >
> > -       dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
> > -       if (mrioc->facts.max_devhandle % 8)
> > -               dev_handle_bitmap_sz++;
> > -       if (dev_handle_bitmap_sz > mrioc->dev_handle_bitmap_sz) {
> > -               removepend_bitmap = krealloc(mrioc->removepend_bitmap,
> > -                   dev_handle_bitmap_sz, GFP_KERNEL);
> > +       if (mrioc->facts.max_devhandle > mrioc->dev_handle_bitmap_bits) {
> >>Free the existing removepend_bitmap prior the alloc.

Thanks for catching this. The existing removepend_bitmap should be freed. I
think the free should be done after the alloc, since the alloc may fail. I'll
add bitmap_free after the bitmap_zalloc() result check.

> > +               removepend_bitmap = bitmap_zalloc(mrioc->facts.max_devhandle,
> > +                                                 GFP_KERNEL);
> >                 if (!removepend_bitmap) {
> >                         ioc_err(mrioc,
> > -                           "failed to increase removepend_bitmap sz from: %d to %d\n",
> > -                           mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
> > +                               "failed to increase removepend_bitmap bits from %d to %d\n",
> > +                               mrioc->dev_handle_bitmap_bits,
> > +                               mrioc->facts.max_devhandle);
> >                         return -EPERM;
> >                 }
> > -               memset(removepend_bitmap + mrioc->dev_handle_bitmap_sz, 0,
> > -                   dev_handle_bitmap_sz - mrioc->dev_handle_bitmap_sz);
> > +               bitmap_copy(removepend_bitmap, mrioc->removepend_bitmap,
> > +                           mrioc->dev_handle_bitmap_bits);
> >>This copy is not needed as the data in the removepend_bitmap is not valid after reset and the zalloc already cleared the memory.

Okay, will remove it.

-- 
Shin'ichiro Kawasaki

Re: [PATCH v4 2/5] scsi: mpi3mr: fix alltgt_info copy size

[Shinichiro Kawasaki]

On Feb 09, 2023 / 10:51, Sathya Prakash Veerichetty wrote:
> On Thu, Jan 26, 2023 at 11:35 PM Shin'ichiro Kawasaki
> <shinichiro.kawasaki@wdc.com> wrote:
> >
> > The function mpi3mr_get_all_tgt_info calculates min_entrylen which holds
> > the valid entry length in alltgt_info. However, it does not refer
> > min_entrylen when it calls sg_copy_from_buffer to copy the valid entries
> > from alltgt_info to job->request_payload. Instead, it specifies the
> > payload length which is larger than the alltgt_info size, then it causes
> > "BUG: KASAN: slab-out-of-bounds". Fix the BUG by specifying the correct
> > length referring the calculated min_entrylen.
> 
> >>both this and the first patch could have been merged. We will do some more cleanup on this function and provide a new patch, we can hold 1 and 2 for now.

I see. I will squash the first patch and the second patch in v5 series.

-- 
Shin'ichiro Kawasaki