Skip to content

Commit 38803fc

Browse files
jsmart-ghChristoph Hellwig
authored andcommitted
nvme-fcloop: fix deallocation of working context
There's been a longstanding bug of LS completions which freed ls ops, particularly the disconnect LS, while executing on a work context that is in the memory being free. Not a good thing to do. Rework LS handling to make callbacks in the rport context rather than the ls_request context. Signed-off-by: James Smart <[email protected]> Reviewed-by: Himanshu Madhani <[email protected]> Reviewed-by: Hannes Reinecke <[email protected]> Signed-off-by: Christoph Hellwig <[email protected]>
1 parent c95b708 commit 38803fc

File tree

1 file changed

+52
-24
lines changed

1 file changed

+52
-24
lines changed

drivers/nvme/target/fcloop.c

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,13 @@ struct fcloop_lport_priv {
198198
};
199199

200200
struct fcloop_rport {
201-
struct nvme_fc_remote_port *remoteport;
202-
struct nvmet_fc_target_port *targetport;
203-
struct fcloop_nport *nport;
204-
struct fcloop_lport *lport;
201+
struct nvme_fc_remote_port *remoteport;
202+
struct nvmet_fc_target_port *targetport;
203+
struct fcloop_nport *nport;
204+
struct fcloop_lport *lport;
205+
spinlock_t lock;
206+
struct list_head ls_list;
207+
struct work_struct ls_work;
205208
};
206209

207210
struct fcloop_tport {
@@ -224,11 +227,10 @@ struct fcloop_nport {
224227
};
225228

226229
struct fcloop_lsreq {
227-
struct fcloop_tport *tport;
228230
struct nvmefc_ls_req *lsreq;
229-
struct work_struct work;
230231
struct nvmefc_tgt_ls_req tgt_ls_req;
231232
int status;
233+
struct list_head ls_list; /* fcloop_rport->ls_list */
232234
};
233235

234236
struct fcloop_rscn {
@@ -292,21 +294,32 @@ fcloop_delete_queue(struct nvme_fc_local_port *localport,
292294
{
293295
}
294296

295-
296-
/*
297-
* Transmit of LS RSP done (e.g. buffers all set). call back up
298-
* initiator "done" flows.
299-
*/
300297
static void
301-
fcloop_tgt_lsrqst_done_work(struct work_struct *work)
298+
fcloop_rport_lsrqst_work(struct work_struct *work)
302299
{
303-
struct fcloop_lsreq *tls_req =
304-
container_of(work, struct fcloop_lsreq, work);
305-
struct fcloop_tport *tport = tls_req->tport;
306-
struct nvmefc_ls_req *lsreq = tls_req->lsreq;
300+
struct fcloop_rport *rport =
301+
container_of(work, struct fcloop_rport, ls_work);
302+
struct fcloop_lsreq *tls_req;
307303

308-
if (!tport || tport->remoteport)
309-
lsreq->done(lsreq, tls_req->status);
304+
spin_lock(&rport->lock);
305+
for (;;) {
306+
tls_req = list_first_entry_or_null(&rport->ls_list,
307+
struct fcloop_lsreq, ls_list);
308+
if (!tls_req)
309+
break;
310+
311+
list_del(&tls_req->ls_list);
312+
spin_unlock(&rport->lock);
313+
314+
tls_req->lsreq->done(tls_req->lsreq, tls_req->status);
315+
/*
316+
* callee may free memory containing tls_req.
317+
* do not reference lsreq after this.
318+
*/
319+
320+
spin_lock(&rport->lock);
321+
}
322+
spin_unlock(&rport->lock);
310323
}
311324

312325
static int
@@ -319,36 +332,47 @@ fcloop_ls_req(struct nvme_fc_local_port *localport,
319332
int ret = 0;
320333

321334
tls_req->lsreq = lsreq;
322-
INIT_WORK(&tls_req->work, fcloop_tgt_lsrqst_done_work);
335+
INIT_LIST_HEAD(&tls_req->ls_list);
323336

324337
if (!rport->targetport) {
325338
tls_req->status = -ECONNREFUSED;
326-
tls_req->tport = NULL;
327-
schedule_work(&tls_req->work);
339+
spin_lock(&rport->lock);
340+
list_add_tail(&rport->ls_list, &tls_req->ls_list);
341+
spin_unlock(&rport->lock);
342+
schedule_work(&rport->ls_work);
328343
return ret;
329344
}
330345

331346
tls_req->status = 0;
332-
tls_req->tport = rport->targetport->private;
333347
ret = nvmet_fc_rcv_ls_req(rport->targetport, &tls_req->tgt_ls_req,
334348
lsreq->rqstaddr, lsreq->rqstlen);
335349

336350
return ret;
337351
}
338352

339353
static int
340-
fcloop_xmt_ls_rsp(struct nvmet_fc_target_port *tport,
354+
fcloop_xmt_ls_rsp(struct nvmet_fc_target_port *targetport,
341355
struct nvmefc_tgt_ls_req *tgt_lsreq)
342356
{
343357
struct fcloop_lsreq *tls_req = tgt_ls_req_to_lsreq(tgt_lsreq);
344358
struct nvmefc_ls_req *lsreq = tls_req->lsreq;
359+
struct fcloop_tport *tport = targetport->private;
360+
struct nvme_fc_remote_port *remoteport = tport->remoteport;
361+
struct fcloop_rport *rport;
345362

346363
memcpy(lsreq->rspaddr, tgt_lsreq->rspbuf,
347364
((lsreq->rsplen < tgt_lsreq->rsplen) ?
348365
lsreq->rsplen : tgt_lsreq->rsplen));
366+
349367
tgt_lsreq->done(tgt_lsreq);
350368

351-
schedule_work(&tls_req->work);
369+
if (remoteport) {
370+
rport = remoteport->private;
371+
spin_lock(&rport->lock);
372+
list_add_tail(&rport->ls_list, &tls_req->ls_list);
373+
spin_unlock(&rport->lock);
374+
schedule_work(&rport->ls_work);
375+
}
352376

353377
return 0;
354378
}
@@ -834,6 +858,7 @@ fcloop_remoteport_delete(struct nvme_fc_remote_port *remoteport)
834858
{
835859
struct fcloop_rport *rport = remoteport->private;
836860

861+
flush_work(&rport->ls_work);
837862
fcloop_nport_put(rport->nport);
838863
}
839864

@@ -1136,6 +1161,9 @@ fcloop_create_remote_port(struct device *dev, struct device_attribute *attr,
11361161
rport->nport = nport;
11371162
rport->lport = nport->lport;
11381163
nport->rport = rport;
1164+
spin_lock_init(&rport->lock);
1165+
INIT_WORK(&rport->ls_work, fcloop_rport_lsrqst_work);
1166+
INIT_LIST_HEAD(&rport->ls_list);
11391167

11401168
return count;
11411169
}

0 commit comments

Comments
 (0)