* [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe
@ 2019-01-17 16:14 Ewan D. Milne
2019-01-17 16:14 ` [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport Ewan D. Milne
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ewan D. Milne @ 2019-01-17 16:14 UTC (permalink / raw)
These two patches fix use-after-free errors in the shutdown path of the lpfc
driver with both the Initiator and Target mode usage of FC ports.
The problem is very apparent with slub_debug enabled, as the memory poisoning
prevents the wait_for_completion_timeout() from returning after the object
has been freed.
Ewan D. Milne (2):
lpfc: nvme: avoid hang / use-after-free when destroying localport
lpfc: nvmet: avoid hang / use-after-free when destroying targetport
drivers/scsi/lpfc/lpfc_nvme.c | 16 +++++++++-------
drivers/scsi/lpfc/lpfc_nvme.h | 2 +-
drivers/scsi/lpfc/lpfc_nvmet.c | 8 +++++---
drivers/scsi/lpfc/lpfc_nvmet.h | 2 +-
4 files changed, 16 insertions(+), 12 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport
2019-01-17 16:14 [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Ewan D. Milne
@ 2019-01-17 16:14 ` Ewan D. Milne
2019-01-17 17:17 ` James Smart
2019-01-17 16:14 ` [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport Ewan D. Milne
2019-01-23 1:43 ` [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Martin K. Petersen
2 siblings, 1 reply; 6+ messages in thread
From: Ewan D. Milne @ 2019-01-17 16:14 UTC (permalink / raw)
We cannot wait on a completion object in the lpfc_nvme_lport structure
in the _destroy_localport() code path because the NVMe/fc transport will
free that structure immediately after the .localport_delete() callback.
This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne <emilne at redhat.com>
---
drivers/scsi/lpfc/lpfc_nvme.c | 16 +++++++++-------
drivers/scsi/lpfc/lpfc_nvme.h | 2 +-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 4c66b19..8c9f790 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -297,7 +297,8 @@ lpfc_nvme_localport_delete(struct nvme_fc_local_port *localport)
lport);
/* release any threads waiting for the unreg to complete */
- complete(&lport->lport_unreg_done);
+ if (lport->vport->localport)
+ complete(lport->lport_unreg_cmp);
}
/* lpfc_nvme_remoteport_delete
@@ -2545,7 +2546,8 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
*/
void
lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
- struct lpfc_nvme_lport *lport)
+ struct lpfc_nvme_lport *lport,
+ struct completion *lport_unreg_cmp)
{
#if (IS_ENABLED(CONFIG_NVME_FC))
u32 wait_tmo;
@@ -2557,8 +2559,7 @@ lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
*/
wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000);
while (true) {
- ret = wait_for_completion_timeout(&lport->lport_unreg_done,
- wait_tmo);
+ ret = wait_for_completion_timeout(lport_unreg_cmp, wait_tmo);
if (unlikely(!ret)) {
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR,
"6176 Lport %p Localport %p wait "
@@ -2592,12 +2593,12 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_ctrl_stat *cstat;
int ret;
+ DECLARE_COMPLETION_ONSTACK(lport_unreg_cmp);
if (vport->nvmei_support == 0)
return;
localport = vport->localport;
- vport->localport = NULL;
lport = (struct lpfc_nvme_lport *)localport->private;
cstat = lport->cstat;
@@ -2608,13 +2609,14 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
/* lport's rport list is clear. Unregister
* lport and release resources.
*/
- init_completion(&lport->lport_unreg_done);
+ lport->lport_unreg_cmp = &lport_unreg_cmp;
ret = nvme_fc_unregister_localport(localport);
/* Wait for completion. This either blocks
* indefinitely or succeeds
*/
- lpfc_nvme_lport_unreg_wait(vport, lport);
+ lpfc_nvme_lport_unreg_wait(vport, lport, &lport_unreg_cmp);
+ vport->localport = NULL;
kfree(cstat);
/* Regardless of the unregister upcall response, clear
diff --git a/drivers/scsi/lpfc/lpfc_nvme.h b/drivers/scsi/lpfc/lpfc_nvme.h
index cfd4719..b234d02 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.h
+++ b/drivers/scsi/lpfc/lpfc_nvme.h
@@ -50,7 +50,7 @@ struct lpfc_nvme_ctrl_stat {
/* Declare nvme-based local and remote port definitions. */
struct lpfc_nvme_lport {
struct lpfc_vport *vport;
- struct completion lport_unreg_done;
+ struct completion *lport_unreg_cmp;
/* Add stats counters here */
struct lpfc_nvme_ctrl_stat *cstat;
atomic_t fc4NvmeLsRequests;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport
2019-01-17 16:14 [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Ewan D. Milne
2019-01-17 16:14 ` [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport Ewan D. Milne
@ 2019-01-17 16:14 ` Ewan D. Milne
2019-01-17 17:17 ` James Smart
2019-01-23 1:43 ` [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Martin K. Petersen
2 siblings, 1 reply; 6+ messages in thread
From: Ewan D. Milne @ 2019-01-17 16:14 UTC (permalink / raw)
We cannot wait on a completion object in the lpfc_nvme_targetport structure
in the _destroy_targetport() code path because the NVMe/fc transport will
free that structure immediately after the .targetport_delete() callback.
This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne <emilne at redhat.com>
---
drivers/scsi/lpfc/lpfc_nvmet.c | 8 +++++---
drivers/scsi/lpfc/lpfc_nvmet.h | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index 6245f44..95fee83 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -1003,7 +1003,8 @@ lpfc_nvmet_targetport_delete(struct nvmet_fc_target_port *targetport)
struct lpfc_nvmet_tgtport *tport = targetport->private;
/* release any threads waiting for the unreg to complete */
- complete(&tport->tport_unreg_done);
+ if (tport->phba->targetport)
+ complete(tport->tport_unreg_cmp);
}
static void
@@ -1692,6 +1693,7 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
struct lpfc_nvmet_tgtport *tgtp;
struct lpfc_queue *wq;
uint32_t qidx;
+ DECLARE_COMPLETION_ONSTACK(tport_unreg_cmp);
if (phba->nvmet_support == 0)
return;
@@ -1701,9 +1703,9 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
wq = phba->sli4_hba.nvme_wq[qidx];
lpfc_nvmet_wqfull_flush(phba, wq, NULL);
}
- init_completion(&tgtp->tport_unreg_done);
+ tgtp->tport_unreg_cmp = &tport_unreg_cmp;
nvmet_fc_unregister_targetport(phba->targetport);
- wait_for_completion_timeout(&tgtp->tport_unreg_done, 5);
+ wait_for_completion_timeout(&tport_unreg_cmp, 5);
lpfc_nvmet_cleanup_io_context(phba);
}
phba->targetport = NULL;
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.h b/drivers/scsi/lpfc/lpfc_nvmet.h
index 1aaff63..0ec1082 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.h
+++ b/drivers/scsi/lpfc/lpfc_nvmet.h
@@ -34,7 +34,7 @@
/* Used for NVME Target */
struct lpfc_nvmet_tgtport {
struct lpfc_hba *phba;
- struct completion tport_unreg_done;
+ struct completion *tport_unreg_cmp;
/* Stats counters - lpfc_nvmet_unsol_ls_buffer */
atomic_t rcv_ls_req_in;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport
2019-01-17 16:14 ` [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport Ewan D. Milne
@ 2019-01-17 17:17 ` James Smart
0 siblings, 0 replies; 6+ messages in thread
From: James Smart @ 2019-01-17 17:17 UTC (permalink / raw)
On 1/17/2019 8:14 AM, Ewan D. Milne wrote:
> We cannot wait on a completion object in the lpfc_nvme_lport structure
> in the _destroy_localport() code path because the NVMe/fc transport will
> free that structure immediately after the .localport_delete() callback.
> This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
>
> Fix this by putting the completion on the stack.
>
> Signed-off-by: Ewan D. Milne <emilne at redhat.com>
> ---
> drivers/scsi/lpfc/lpfc_nvme.c | 16 +++++++++-------
> drivers/scsi/lpfc/lpfc_nvme.h | 2 +-
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
>
Reviewed-by:?? James Smart? <james.smart at broadcom.com>
Thank you Ewan
-- james
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport
2019-01-17 16:14 ` [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport Ewan D. Milne
@ 2019-01-17 17:17 ` James Smart
0 siblings, 0 replies; 6+ messages in thread
From: James Smart @ 2019-01-17 17:17 UTC (permalink / raw)
On 1/17/2019 8:14 AM, Ewan D. Milne wrote:
> We cannot wait on a completion object in the lpfc_nvme_targetport structure
> in the _destroy_targetport() code path because the NVMe/fc transport will
> free that structure immediately after the .targetport_delete() callback.
> This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
>
> Fix this by putting the completion on the stack.
>
> Signed-off-by: Ewan D. Milne <emilne at redhat.com>
> ---
> drivers/scsi/lpfc/lpfc_nvmet.c | 8 +++++---
> drivers/scsi/lpfc/lpfc_nvmet.h | 2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
>
Reviewed-by:?? James Smart? <james.smart at broadcom.com>
Thank you Ewan
-- james
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe
2019-01-17 16:14 [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Ewan D. Milne
2019-01-17 16:14 ` [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport Ewan D. Milne
2019-01-17 16:14 ` [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport Ewan D. Milne
@ 2019-01-23 1:43 ` Martin K. Petersen
2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2019-01-23 1:43 UTC (permalink / raw)
Ewan,
> These two patches fix use-after-free errors in the shutdown path of
> the lpfc driver with both the Initiator and Target mode usage of FC
> ports.
>
> The problem is very apparent with slub_debug enabled, as the memory
> poisoning prevents the wait_for_completion_timeout() from returning
> after the object has been freed.
Applied to 5.0/scsi-fixes. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-23 1:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17 16:14 [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Ewan D. Milne
2019-01-17 16:14 ` [PATCH 1/2] lpfc: nvme: avoid hang / use-after-free when destroying localport Ewan D. Milne
2019-01-17 17:17 ` James Smart
2019-01-17 16:14 ` [PATCH 2/2] lpfc: nvmet: avoid hang / use-after-free when destroying targetport Ewan D. Milne
2019-01-17 17:17 ` James Smart
2019-01-23 1:43 ` [PATCH 0/2] Fix use-after-free errors in lpfc driver when using NVMe Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.