Skip to content

Commit b39f2d1

Browse files
tyreldmartinkpetersen
authored andcommitted
scsi: ibmvfc: Remove BUG_ON in the case of an empty event pool
In practice the driver should never send more commands than are allocated to a queue's event pool. In the unlikely event that this happens, the code asserts a BUG_ON, and in the case that the kernel is not configured to crash on panic returns a junk event pointer from the empty event list causing things to spiral from there. This BUG_ON is a historical artifact of the ibmvfc driver first being upstreamed, and it is well known now that the use of BUG_ON is bad practice except in the most unrecoverable scenario. There is nothing about this scenario that prevents the driver from recovering and carrying on. Remove the BUG_ON in question from ibmvfc_get_event() and return a NULL pointer in the case of an empty event pool. Update all call sites to ibmvfc_get_event() to check for a NULL pointer and perfrom the appropriate failure or recovery action. Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com> Link: https://lore.kernel.org/r/20230921225435.3537728-2-tyreld@linux.ibm.com Reviewed-by: Brian King <brking@linux.vnet.ibm.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 0bb80ec commit b39f2d1

1 file changed

Lines changed: 122 additions & 2 deletions

File tree

drivers/scsi/ibmvscsi/ibmvfc.c

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,11 @@ static struct ibmvfc_event *ibmvfc_get_event(struct ibmvfc_queue *queue)
15191519
unsigned long flags;
15201520

15211521
spin_lock_irqsave(&queue->l_lock, flags);
1522-
BUG_ON(list_empty(&queue->free));
1522+
if (list_empty(&queue->free)) {
1523+
ibmvfc_log(queue->vhost, 4, "empty event pool on queue:%ld\n", queue->hwq_id);
1524+
spin_unlock_irqrestore(&queue->l_lock, flags);
1525+
return NULL;
1526+
}
15231527
evt = list_entry(queue->free.next, struct ibmvfc_event, queue_list);
15241528
atomic_set(&evt->free, 0);
15251529
list_del(&evt->queue_list);
@@ -1948,9 +1952,15 @@ static int ibmvfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
19481952
if (vhost->using_channels) {
19491953
scsi_channel = hwq % vhost->scsi_scrqs.active_queues;
19501954
evt = ibmvfc_get_event(&vhost->scsi_scrqs.scrqs[scsi_channel]);
1955+
if (!evt)
1956+
return SCSI_MLQUEUE_HOST_BUSY;
1957+
19511958
evt->hwq = hwq % vhost->scsi_scrqs.active_queues;
1952-
} else
1959+
} else {
19531960
evt = ibmvfc_get_event(&vhost->crq);
1961+
if (!evt)
1962+
return SCSI_MLQUEUE_HOST_BUSY;
1963+
}
19541964

19551965
ibmvfc_init_event(evt, ibmvfc_scsi_done, IBMVFC_CMD_FORMAT);
19561966
evt->cmnd = cmnd;
@@ -2038,6 +2048,11 @@ static int ibmvfc_bsg_timeout(struct bsg_job *job)
20382048

20392049
vhost->aborting_passthru = 1;
20402050
evt = ibmvfc_get_event(&vhost->crq);
2051+
if (!evt) {
2052+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
2053+
return -ENOMEM;
2054+
}
2055+
20412056
ibmvfc_init_event(evt, ibmvfc_bsg_timeout_done, IBMVFC_MAD_FORMAT);
20422057

20432058
tmf = &evt->iu.tmf;
@@ -2096,6 +2111,10 @@ static int ibmvfc_bsg_plogi(struct ibmvfc_host *vhost, unsigned int port_id)
20962111
goto unlock_out;
20972112

20982113
evt = ibmvfc_get_event(&vhost->crq);
2114+
if (!evt) {
2115+
rc = -ENOMEM;
2116+
goto unlock_out;
2117+
}
20992118
ibmvfc_init_event(evt, ibmvfc_sync_completion, IBMVFC_MAD_FORMAT);
21002119
plogi = &evt->iu.plogi;
21012120
memset(plogi, 0, sizeof(*plogi));
@@ -2214,6 +2233,11 @@ static int ibmvfc_bsg_request(struct bsg_job *job)
22142233
}
22152234

22162235
evt = ibmvfc_get_event(&vhost->crq);
2236+
if (!evt) {
2237+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
2238+
rc = -ENOMEM;
2239+
goto out;
2240+
}
22172241
ibmvfc_init_event(evt, ibmvfc_sync_completion, IBMVFC_MAD_FORMAT);
22182242
mad = &evt->iu.passthru;
22192243

@@ -2302,6 +2326,11 @@ static int ibmvfc_reset_device(struct scsi_device *sdev, int type, char *desc)
23022326
else
23032327
evt = ibmvfc_get_event(&vhost->crq);
23042328

2329+
if (!evt) {
2330+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
2331+
return -ENOMEM;
2332+
}
2333+
23052334
ibmvfc_init_event(evt, ibmvfc_sync_completion, IBMVFC_CMD_FORMAT);
23062335
tmf = ibmvfc_init_vfc_cmd(evt, sdev);
23072336
iu = ibmvfc_get_fcp_iu(vhost, tmf);
@@ -2505,6 +2534,8 @@ static struct ibmvfc_event *ibmvfc_init_tmf(struct ibmvfc_queue *queue,
25052534
struct ibmvfc_tmf *tmf;
25062535

25072536
evt = ibmvfc_get_event(queue);
2537+
if (!evt)
2538+
return NULL;
25082539
ibmvfc_init_event(evt, ibmvfc_sync_completion, IBMVFC_MAD_FORMAT);
25092540

25102541
tmf = &evt->iu.tmf;
@@ -2561,6 +2592,11 @@ static int ibmvfc_cancel_all_mq(struct scsi_device *sdev, int type)
25612592

25622593
if (found_evt && vhost->logged_in) {
25632594
evt = ibmvfc_init_tmf(&queues[i], sdev, type);
2595+
if (!evt) {
2596+
spin_unlock(queues[i].q_lock);
2597+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
2598+
return -ENOMEM;
2599+
}
25642600
evt->sync_iu = &queues[i].cancel_rsp;
25652601
ibmvfc_send_event(evt, vhost, default_timeout);
25662602
list_add_tail(&evt->cancel, &cancelq);
@@ -2774,6 +2810,10 @@ static int ibmvfc_abort_task_set(struct scsi_device *sdev)
27742810

27752811
if (vhost->state == IBMVFC_ACTIVE) {
27762812
evt = ibmvfc_get_event(&vhost->crq);
2813+
if (!evt) {
2814+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
2815+
return -ENOMEM;
2816+
}
27772817
ibmvfc_init_event(evt, ibmvfc_sync_completion, IBMVFC_CMD_FORMAT);
27782818
tmf = ibmvfc_init_vfc_cmd(evt, sdev);
27792819
iu = ibmvfc_get_fcp_iu(vhost, tmf);
@@ -4032,6 +4072,12 @@ static void ibmvfc_tgt_send_prli(struct ibmvfc_target *tgt)
40324072

40334073
kref_get(&tgt->kref);
40344074
evt = ibmvfc_get_event(&vhost->crq);
4075+
if (!evt) {
4076+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
4077+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4078+
__ibmvfc_reset_host(vhost);
4079+
return;
4080+
}
40354081
vhost->discovery_threads++;
40364082
ibmvfc_init_event(evt, ibmvfc_tgt_prli_done, IBMVFC_MAD_FORMAT);
40374083
evt->tgt = tgt;
@@ -4139,6 +4185,12 @@ static void ibmvfc_tgt_send_plogi(struct ibmvfc_target *tgt)
41394185
kref_get(&tgt->kref);
41404186
tgt->logo_rcvd = 0;
41414187
evt = ibmvfc_get_event(&vhost->crq);
4188+
if (!evt) {
4189+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
4190+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4191+
__ibmvfc_reset_host(vhost);
4192+
return;
4193+
}
41424194
vhost->discovery_threads++;
41434195
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_INIT_WAIT);
41444196
ibmvfc_init_event(evt, ibmvfc_tgt_plogi_done, IBMVFC_MAD_FORMAT);
@@ -4215,6 +4267,8 @@ static struct ibmvfc_event *__ibmvfc_tgt_get_implicit_logout_evt(struct ibmvfc_t
42154267

42164268
kref_get(&tgt->kref);
42174269
evt = ibmvfc_get_event(&vhost->crq);
4270+
if (!evt)
4271+
return NULL;
42184272
ibmvfc_init_event(evt, done, IBMVFC_MAD_FORMAT);
42194273
evt->tgt = tgt;
42204274
mad = &evt->iu.implicit_logout;
@@ -4242,6 +4296,13 @@ static void ibmvfc_tgt_implicit_logout(struct ibmvfc_target *tgt)
42424296
vhost->discovery_threads++;
42434297
evt = __ibmvfc_tgt_get_implicit_logout_evt(tgt,
42444298
ibmvfc_tgt_implicit_logout_done);
4299+
if (!evt) {
4300+
vhost->discovery_threads--;
4301+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
4302+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4303+
__ibmvfc_reset_host(vhost);
4304+
return;
4305+
}
42454306

42464307
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_INIT_WAIT);
42474308
if (ibmvfc_send_event(evt, vhost, default_timeout)) {
@@ -4381,6 +4442,12 @@ static void ibmvfc_tgt_move_login(struct ibmvfc_target *tgt)
43814442

43824443
kref_get(&tgt->kref);
43834444
evt = ibmvfc_get_event(&vhost->crq);
4445+
if (!evt) {
4446+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_DEL_RPORT);
4447+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4448+
__ibmvfc_reset_host(vhost);
4449+
return;
4450+
}
43844451
vhost->discovery_threads++;
43854452
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_INIT_WAIT);
43864453
ibmvfc_init_event(evt, ibmvfc_tgt_move_login_done, IBMVFC_MAD_FORMAT);
@@ -4547,6 +4614,14 @@ static void ibmvfc_adisc_timeout(struct timer_list *t)
45474614
vhost->abort_threads++;
45484615
kref_get(&tgt->kref);
45494616
evt = ibmvfc_get_event(&vhost->crq);
4617+
if (!evt) {
4618+
tgt_err(tgt, "Failed to get cancel event for ADISC.\n");
4619+
vhost->abort_threads--;
4620+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4621+
__ibmvfc_reset_host(vhost);
4622+
spin_unlock_irqrestore(vhost->host->host_lock, flags);
4623+
return;
4624+
}
45504625
ibmvfc_init_event(evt, ibmvfc_tgt_adisc_cancel_done, IBMVFC_MAD_FORMAT);
45514626

45524627
evt->tgt = tgt;
@@ -4597,6 +4672,12 @@ static void ibmvfc_tgt_adisc(struct ibmvfc_target *tgt)
45974672

45984673
kref_get(&tgt->kref);
45994674
evt = ibmvfc_get_event(&vhost->crq);
4675+
if (!evt) {
4676+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
4677+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4678+
__ibmvfc_reset_host(vhost);
4679+
return;
4680+
}
46004681
vhost->discovery_threads++;
46014682
ibmvfc_init_event(evt, ibmvfc_tgt_adisc_done, IBMVFC_MAD_FORMAT);
46024683
evt->tgt = tgt;
@@ -4700,6 +4781,12 @@ static void ibmvfc_tgt_query_target(struct ibmvfc_target *tgt)
47004781

47014782
kref_get(&tgt->kref);
47024783
evt = ibmvfc_get_event(&vhost->crq);
4784+
if (!evt) {
4785+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
4786+
kref_put(&tgt->kref, ibmvfc_release_tgt);
4787+
__ibmvfc_reset_host(vhost);
4788+
return;
4789+
}
47034790
vhost->discovery_threads++;
47044791
evt->tgt = tgt;
47054792
ibmvfc_init_event(evt, ibmvfc_tgt_query_target_done, IBMVFC_MAD_FORMAT);
@@ -4872,6 +4959,13 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
48724959
{
48734960
struct ibmvfc_discover_targets *mad;
48744961
struct ibmvfc_event *evt = ibmvfc_get_event(&vhost->crq);
4962+
int level = IBMVFC_DEFAULT_LOG_LEVEL;
4963+
4964+
if (!evt) {
4965+
ibmvfc_log(vhost, level, "Discover Targets failed: no available events\n");
4966+
ibmvfc_hard_reset_host(vhost);
4967+
return;
4968+
}
48754969

48764970
ibmvfc_init_event(evt, ibmvfc_discover_targets_done, IBMVFC_MAD_FORMAT);
48774971
mad = &evt->iu.discover_targets;
@@ -4949,8 +5043,15 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
49495043
struct ibmvfc_scsi_channels *scrqs = &vhost->scsi_scrqs;
49505044
unsigned int num_channels =
49515045
min(vhost->client_scsi_channels, vhost->max_vios_scsi_channels);
5046+
int level = IBMVFC_DEFAULT_LOG_LEVEL;
49525047
int i;
49535048

5049+
if (!evt) {
5050+
ibmvfc_log(vhost, level, "Channel Setup failed: no available events\n");
5051+
ibmvfc_hard_reset_host(vhost);
5052+
return;
5053+
}
5054+
49545055
memset(setup_buf, 0, sizeof(*setup_buf));
49555056
if (num_channels == 0)
49565057
setup_buf->flags = cpu_to_be32(IBMVFC_CANCEL_CHANNELS);
@@ -5012,6 +5113,13 @@ static void ibmvfc_channel_enquiry(struct ibmvfc_host *vhost)
50125113
{
50135114
struct ibmvfc_channel_enquiry *mad;
50145115
struct ibmvfc_event *evt = ibmvfc_get_event(&vhost->crq);
5116+
int level = IBMVFC_DEFAULT_LOG_LEVEL;
5117+
5118+
if (!evt) {
5119+
ibmvfc_log(vhost, level, "Channel Enquiry failed: no available events\n");
5120+
ibmvfc_hard_reset_host(vhost);
5121+
return;
5122+
}
50155123

50165124
ibmvfc_init_event(evt, ibmvfc_channel_enquiry_done, IBMVFC_MAD_FORMAT);
50175125
mad = &evt->iu.channel_enquiry;
@@ -5134,6 +5242,12 @@ static void ibmvfc_npiv_login(struct ibmvfc_host *vhost)
51345242
struct ibmvfc_npiv_login_mad *mad;
51355243
struct ibmvfc_event *evt = ibmvfc_get_event(&vhost->crq);
51365244

5245+
if (!evt) {
5246+
ibmvfc_dbg(vhost, "NPIV Login failed: no available events\n");
5247+
ibmvfc_hard_reset_host(vhost);
5248+
return;
5249+
}
5250+
51375251
ibmvfc_gather_partition_info(vhost);
51385252
ibmvfc_set_login_info(vhost);
51395253
ibmvfc_init_event(evt, ibmvfc_npiv_login_done, IBMVFC_MAD_FORMAT);
@@ -5198,6 +5312,12 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *vhost)
51985312
struct ibmvfc_event *evt;
51995313

52005314
evt = ibmvfc_get_event(&vhost->crq);
5315+
if (!evt) {
5316+
ibmvfc_dbg(vhost, "NPIV Logout failed: no available events\n");
5317+
ibmvfc_hard_reset_host(vhost);
5318+
return;
5319+
}
5320+
52015321
ibmvfc_init_event(evt, ibmvfc_npiv_logout_done, IBMVFC_MAD_FORMAT);
52025322

52035323
mad = &evt->iu.npiv_logout;

0 commit comments

Comments
 (0)