Skip to content

Commit 339e615

Browse files
kawasakimartinkpetersen
kawasaki
authored and
martinkpetersen
committed
scsi: mpi3mr: Use number of bits to manage bitmap sizes
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() and krealloc(). For memory free, call bitmap_free() instead of kfree(). For zero clear, call bitmap_clear() instead of memset(). 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. Link: https://lore.kernel.org/r/20230214005019.1897251-4-shinichiro.kawasaki@wdc.com Fixes: c5758fc ("scsi: mpi3mr: Gracefully handle online FW update operation") Fixes: e844adb ("scsi: mpi3mr: Implement SCSI error handler hooks") Fixes: c1af985 ("scsi: mpi3mr: Add Event acknowledgment logic") Fixes: 824a156 ("scsi: mpi3mr: Base driver code") Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> Acked-by: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent eeb270a commit 339e615

2 files changed

Lines changed: 33 additions & 52 deletions

File tree

‎drivers/scsi/mpi3mr/mpi3mr.h‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -955,19 +955,16 @@ struct scmd_priv {
955955
* @chain_buf_count: Chain buffer count
956956
* @chain_buf_pool: Chain buffer pool
957957
* @chain_sgl_list: Chain SGL list
958-
* @chain_bitmap_sz: Chain buffer allocator bitmap size
959958
* @chain_bitmap: Chain buffer allocator bitmap
960959
* @chain_buf_lock: Chain buffer list lock
961960
* @bsg_cmds: Command tracker for BSG command
962961
* @host_tm_cmds: Command tracker for task management commands
963962
* @dev_rmhs_cmds: Command tracker for device removal commands
964963
* @evtack_cmds: Command tracker for event ack commands
965-
* @devrem_bitmap_sz: Device removal bitmap size
966964
* @devrem_bitmap: Device removal bitmap
967-
* @dev_handle_bitmap_sz: Device handle bitmap size
965+
* @dev_handle_bitmap_bits: Number of bits in device handle bitmap
968966
* @removepend_bitmap: Remove pending bitmap
969967
* @delayed_rmhs_list: Delayed device removal list
970-
* @evtack_cmds_bitmap_sz: Event Ack bitmap size
971968
* @evtack_cmds_bitmap: Event Ack bitmap
972969
* @delayed_evtack_cmds_list: Delayed event acknowledgment list
973970
* @ts_update_counter: Timestamp update counter
@@ -1128,20 +1125,17 @@ struct mpi3mr_ioc {
11281125
u32 chain_buf_count;
11291126
struct dma_pool *chain_buf_pool;
11301127
struct chain_element *chain_sgl_list;
1131-
u16 chain_bitmap_sz;
11321128
void *chain_bitmap;
11331129
spinlock_t chain_buf_lock;
11341130

11351131
struct mpi3mr_drv_cmd bsg_cmds;
11361132
struct mpi3mr_drv_cmd host_tm_cmds;
11371133
struct mpi3mr_drv_cmd dev_rmhs_cmds[MPI3MR_NUM_DEVRMCMD];
11381134
struct mpi3mr_drv_cmd evtack_cmds[MPI3MR_NUM_EVTACKCMD];
1139-
u16 devrem_bitmap_sz;
11401135
void *devrem_bitmap;
1141-
u16 dev_handle_bitmap_sz;
1136+
u16 dev_handle_bitmap_bits;
11421137
void *removepend_bitmap;
11431138
struct list_head delayed_rmhs_list;
1144-
u16 evtack_cmds_bitmap_sz;
11451139
void *evtack_cmds_bitmap;
11461140
struct list_head delayed_evtack_cmds_list;
11471141

‎drivers/scsi/mpi3mr/mpi3mr_fw.c‎

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,6 @@ static int mpi3mr_issue_and_process_mur(struct mpi3mr_ioc *mrioc,
11281128
static int
11291129
mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
11301130
{
1131-
u16 dev_handle_bitmap_sz;
11321131
void *removepend_bitmap;
11331132

11341133
if (mrioc->facts.reply_sz > mrioc->reply_sz) {
@@ -1160,25 +1159,23 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc)
11601159
"\tcontroller while sas transport support is enabled at the\n"
11611160
"\tdriver, please reboot the system or reload the driver\n");
11621161

1163-
dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
1164-
if (mrioc->facts.max_devhandle % 8)
1165-
dev_handle_bitmap_sz++;
1166-
if (dev_handle_bitmap_sz > mrioc->dev_handle_bitmap_sz) {
1167-
removepend_bitmap = krealloc(mrioc->removepend_bitmap,
1168-
dev_handle_bitmap_sz, GFP_KERNEL);
1162+
if (mrioc->facts.max_devhandle > mrioc->dev_handle_bitmap_bits) {
1163+
removepend_bitmap = bitmap_zalloc(mrioc->facts.max_devhandle,
1164+
GFP_KERNEL);
11691165
if (!removepend_bitmap) {
11701166
ioc_err(mrioc,
1171-
"failed to increase removepend_bitmap sz from: %d to %d\n",
1172-
mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
1167+
"failed to increase removepend_bitmap bits from %d to %d\n",
1168+
mrioc->dev_handle_bitmap_bits,
1169+
mrioc->facts.max_devhandle);
11731170
return -EPERM;
11741171
}
1175-
memset(removepend_bitmap + mrioc->dev_handle_bitmap_sz, 0,
1176-
dev_handle_bitmap_sz - mrioc->dev_handle_bitmap_sz);
1172+
bitmap_free(mrioc->removepend_bitmap);
11771173
mrioc->removepend_bitmap = removepend_bitmap;
11781174
ioc_info(mrioc,
1179-
"increased dev_handle_bitmap_sz from %d to %d\n",
1180-
mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz);
1181-
mrioc->dev_handle_bitmap_sz = dev_handle_bitmap_sz;
1175+
"increased bits of dev_handle_bitmap from %d to %d\n",
1176+
mrioc->dev_handle_bitmap_bits,
1177+
mrioc->facts.max_devhandle);
1178+
mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
11821179
}
11831180

11841181
return 0;
@@ -2957,27 +2954,18 @@ static int mpi3mr_alloc_reply_sense_bufs(struct mpi3mr_ioc *mrioc)
29572954
if (!mrioc->pel_abort_cmd.reply)
29582955
goto out_failed;
29592956

2960-
mrioc->dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8;
2961-
if (mrioc->facts.max_devhandle % 8)
2962-
mrioc->dev_handle_bitmap_sz++;
2963-
mrioc->removepend_bitmap = kzalloc(mrioc->dev_handle_bitmap_sz,
2964-
GFP_KERNEL);
2957+
mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle;
2958+
mrioc->removepend_bitmap = bitmap_zalloc(mrioc->dev_handle_bitmap_bits,
2959+
GFP_KERNEL);
29652960
if (!mrioc->removepend_bitmap)
29662961
goto out_failed;
29672962

2968-
mrioc->devrem_bitmap_sz = MPI3MR_NUM_DEVRMCMD / 8;
2969-
if (MPI3MR_NUM_DEVRMCMD % 8)
2970-
mrioc->devrem_bitmap_sz++;
2971-
mrioc->devrem_bitmap = kzalloc(mrioc->devrem_bitmap_sz,
2972-
GFP_KERNEL);
2963+
mrioc->devrem_bitmap = bitmap_zalloc(MPI3MR_NUM_DEVRMCMD, GFP_KERNEL);
29732964
if (!mrioc->devrem_bitmap)
29742965
goto out_failed;
29752966

2976-
mrioc->evtack_cmds_bitmap_sz = MPI3MR_NUM_EVTACKCMD / 8;
2977-
if (MPI3MR_NUM_EVTACKCMD % 8)
2978-
mrioc->evtack_cmds_bitmap_sz++;
2979-
mrioc->evtack_cmds_bitmap = kzalloc(mrioc->evtack_cmds_bitmap_sz,
2980-
GFP_KERNEL);
2967+
mrioc->evtack_cmds_bitmap = bitmap_zalloc(MPI3MR_NUM_EVTACKCMD,
2968+
GFP_KERNEL);
29812969
if (!mrioc->evtack_cmds_bitmap)
29822970
goto out_failed;
29832971

@@ -3415,10 +3403,7 @@ static int mpi3mr_alloc_chain_bufs(struct mpi3mr_ioc *mrioc)
34153403
if (!mrioc->chain_sgl_list[i].addr)
34163404
goto out_failed;
34173405
}
3418-
mrioc->chain_bitmap_sz = num_chains / 8;
3419-
if (num_chains % 8)
3420-
mrioc->chain_bitmap_sz++;
3421-
mrioc->chain_bitmap = kzalloc(mrioc->chain_bitmap_sz, GFP_KERNEL);
3406+
mrioc->chain_bitmap = bitmap_zalloc(num_chains, GFP_KERNEL);
34223407
if (!mrioc->chain_bitmap)
34233408
goto out_failed;
34243409
return retval;
@@ -4190,10 +4175,11 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc)
41904175
for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++)
41914176
memset(mrioc->evtack_cmds[i].reply, 0,
41924177
sizeof(*mrioc->evtack_cmds[i].reply));
4193-
memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
4194-
memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
4195-
memset(mrioc->evtack_cmds_bitmap, 0,
4196-
mrioc->evtack_cmds_bitmap_sz);
4178+
bitmap_clear(mrioc->removepend_bitmap, 0,
4179+
mrioc->dev_handle_bitmap_bits);
4180+
bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
4181+
bitmap_clear(mrioc->evtack_cmds_bitmap, 0,
4182+
MPI3MR_NUM_EVTACKCMD);
41974183
}
41984184

41994185
for (i = 0; i < mrioc->num_queues; i++) {
@@ -4319,16 +4305,16 @@ void mpi3mr_free_mem(struct mpi3mr_ioc *mrioc)
43194305
mrioc->evtack_cmds[i].reply = NULL;
43204306
}
43214307

4322-
kfree(mrioc->removepend_bitmap);
4308+
bitmap_free(mrioc->removepend_bitmap);
43234309
mrioc->removepend_bitmap = NULL;
43244310

4325-
kfree(mrioc->devrem_bitmap);
4311+
bitmap_free(mrioc->devrem_bitmap);
43264312
mrioc->devrem_bitmap = NULL;
43274313

4328-
kfree(mrioc->evtack_cmds_bitmap);
4314+
bitmap_free(mrioc->evtack_cmds_bitmap);
43294315
mrioc->evtack_cmds_bitmap = NULL;
43304316

4331-
kfree(mrioc->chain_bitmap);
4317+
bitmap_free(mrioc->chain_bitmap);
43324318
mrioc->chain_bitmap = NULL;
43334319

43344320
kfree(mrioc->transport_cmds.reply);
@@ -4887,9 +4873,10 @@ int mpi3mr_soft_reset_handler(struct mpi3mr_ioc *mrioc,
48874873

48884874
mpi3mr_flush_delayed_cmd_lists(mrioc);
48894875
mpi3mr_flush_drv_cmds(mrioc);
4890-
memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz);
4891-
memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz);
4892-
memset(mrioc->evtack_cmds_bitmap, 0, mrioc->evtack_cmds_bitmap_sz);
4876+
bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD);
4877+
bitmap_clear(mrioc->removepend_bitmap, 0,
4878+
mrioc->dev_handle_bitmap_bits);
4879+
bitmap_clear(mrioc->evtack_cmds_bitmap, 0, MPI3MR_NUM_EVTACKCMD);
48934880
mpi3mr_flush_host_io(mrioc);
48944881
mpi3mr_cleanup_fwevt_list(mrioc);
48954882
mpi3mr_invalidate_devhandles(mrioc);

0 commit comments

Comments
 (0)