* [PATCH 1/1] scsi: qedi: Fix host removal with running sessions
@ 2021-06-09 19:27 Mike Christie
2021-06-10 3:07 ` Martin K. Petersen
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Mike Christie @ 2021-06-09 19:27 UTC (permalink / raw)
To: mrangankar, martin.petersen, linux-scsi; +Cc: Mike Christie
qedi_clear_session_ctx could race with the in-kernel or userspace driven
recovery/removal and we could access a NULL conn or do a double free.
We should be using iscsi_host_remove to start the removal process from the
driver. It will start the in-kernel recovery and notify userspace that the
driver's scsi_hosts are being removed. iscsid will then drive the session
removal like is done when the logout command is run. When the sessions are
removed, iscsi_host_remove will return so qedi can finish knowing there
are no running sessions and no new sessions will be allowed.
This also fixes an issue where we check for a NULL conn after already
accessing it introduced in commit 27e986289e73 ("scsi: iscsi: Drop suspend
calls from ep_disconnect") by just removing the function completely.
Fixes: 27e986289e73 ("scsi: iscsi: Drop suspend calls from ep_disconnect")
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
drivers/scsi/qedi/qedi_gbl.h | 1 -
drivers/scsi/qedi/qedi_iscsi.c | 17 -----------------
drivers/scsi/qedi/qedi_main.c | 7 ++-----
3 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
index fb44a282613e..9f8e8ef405a1 100644
--- a/drivers/scsi/qedi/qedi_gbl.h
+++ b/drivers/scsi/qedi/qedi_gbl.h
@@ -72,6 +72,5 @@ void qedi_remove_sysfs_ctx_attr(struct qedi_ctx *qedi);
void qedi_clearsq(struct qedi_ctx *qedi,
struct qedi_conn *qedi_conn,
struct iscsi_task *task);
-void qedi_clear_session_ctx(struct iscsi_cls_session *cls_sess);
#endif
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index bf581ecea897..97f83760da88 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -1659,23 +1659,6 @@ void qedi_process_iscsi_error(struct qedi_endpoint *ep,
qedi_start_conn_recovery(qedi_conn->qedi, qedi_conn);
}
-void qedi_clear_session_ctx(struct iscsi_cls_session *cls_sess)
-{
- struct iscsi_session *session = cls_sess->dd_data;
- struct iscsi_conn *conn = session->leadconn;
- struct qedi_conn *qedi_conn = conn->dd_data;
-
- if (iscsi_is_session_online(cls_sess)) {
- if (conn)
- iscsi_suspend_queue(conn);
- qedi_ep_disconnect(qedi_conn->iscsi_ep);
- }
-
- qedi_conn_destroy(qedi_conn->cls_conn);
-
- qedi_session_destroy(cls_sess);
-}
-
void qedi_process_tcp_error(struct qedi_endpoint *ep,
struct iscsi_eqe_data *data)
{
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index edf915432704..0b0acb827071 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -2417,11 +2417,9 @@ static void __qedi_remove(struct pci_dev *pdev, int mode)
int rval;
u16 retry = 10;
- if (mode == QEDI_MODE_SHUTDOWN)
- iscsi_host_for_each_session(qedi->shost,
- qedi_clear_session_ctx);
-
if (mode == QEDI_MODE_NORMAL || mode == QEDI_MODE_SHUTDOWN) {
+ iscsi_host_remove(qedi->shost);
+
if (qedi->tmf_thread) {
flush_workqueue(qedi->tmf_thread);
destroy_workqueue(qedi->tmf_thread);
@@ -2482,7 +2480,6 @@ static void __qedi_remove(struct pci_dev *pdev, int mode)
if (qedi->boot_kset)
iscsi_boot_destroy_kset(qedi->boot_kset);
- iscsi_host_remove(qedi->shost);
iscsi_host_free(qedi->shost);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] scsi: qedi: Fix host removal with running sessions
2021-06-09 19:27 [PATCH 1/1] scsi: qedi: Fix host removal with running sessions Mike Christie
@ 2021-06-10 3:07 ` Martin K. Petersen
2021-06-10 4:44 ` [EXT] " Manish Rangankar
2021-06-16 3:49 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2021-06-10 3:07 UTC (permalink / raw)
To: Mike Christie; +Cc: mrangankar, martin.petersen, linux-scsi
Mike,
> qedi_clear_session_ctx could race with the in-kernel or userspace
> driven recovery/removal and we could access a NULL conn or do a double
> free.
Applied to 5.14/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [EXT] [PATCH 1/1] scsi: qedi: Fix host removal with running sessions
2021-06-09 19:27 [PATCH 1/1] scsi: qedi: Fix host removal with running sessions Mike Christie
2021-06-10 3:07 ` Martin K. Petersen
@ 2021-06-10 4:44 ` Manish Rangankar
2021-06-16 3:49 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Manish Rangankar @ 2021-06-10 4:44 UTC (permalink / raw)
To: Mike Christie, martin.petersen, linux-scsi
> -----Original Message-----
> From: Mike Christie <michael.christie@oracle.com>
> Sent: Thursday, June 10, 2021 12:57 AM
> To: Manish Rangankar <mrangankar@marvell.com>;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mike Christie <michael.christie@oracle.com>
> Subject: [EXT] [PATCH 1/1] scsi: qedi: Fix host removal with running sessions
>
> External Email
>
> ----------------------------------------------------------------------
> qedi_clear_session_ctx could race with the in-kernel or userspace driven
> recovery/removal and we could access a NULL conn or do a double free.
>
> We should be using iscsi_host_remove to start the removal process from the
> driver. It will start the in-kernel recovery and notify userspace that the driver's
> scsi_hosts are being removed. iscsid will then drive the session removal like is
> done when the logout command is run. When the sessions are removed,
> iscsi_host_remove will return so qedi can finish knowing there are no running
> sessions and no new sessions will be allowed.
>
> This also fixes an issue where we check for a NULL conn after already accessing
> it introduced in commit 27e986289e73 ("scsi: iscsi: Drop suspend calls from
> ep_disconnect") by just removing the function completely.
>
> Fixes: 27e986289e73 ("scsi: iscsi: Drop suspend calls from ep_disconnect")
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
> drivers/scsi/qedi/qedi_gbl.h | 1 -
> drivers/scsi/qedi/qedi_iscsi.c | 17 ----------------- drivers/scsi/qedi/qedi_main.c
> | 7 ++-----
> 3 files changed, 2 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h index
> fb44a282613e..9f8e8ef405a1 100644
> --- a/drivers/scsi/qedi/qedi_gbl.h
> +++ b/drivers/scsi/qedi/qedi_gbl.h
> @@ -72,6 +72,5 @@ void qedi_remove_sysfs_ctx_attr(struct qedi_ctx *qedi);
> void qedi_clearsq(struct qedi_ctx *qedi,
> struct qedi_conn *qedi_conn,
> struct iscsi_task *task);
> -void qedi_clear_session_ctx(struct iscsi_cls_session *cls_sess);
>
> #endif
> diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c index
> bf581ecea897..97f83760da88 100644
> --- a/drivers/scsi/qedi/qedi_iscsi.c
> +++ b/drivers/scsi/qedi/qedi_iscsi.c
> @@ -1659,23 +1659,6 @@ void qedi_process_iscsi_error(struct qedi_endpoint
> *ep,
> qedi_start_conn_recovery(qedi_conn->qedi, qedi_conn); }
>
> -void qedi_clear_session_ctx(struct iscsi_cls_session *cls_sess) -{
> - struct iscsi_session *session = cls_sess->dd_data;
> - struct iscsi_conn *conn = session->leadconn;
> - struct qedi_conn *qedi_conn = conn->dd_data;
> -
> - if (iscsi_is_session_online(cls_sess)) {
> - if (conn)
> - iscsi_suspend_queue(conn);
> - qedi_ep_disconnect(qedi_conn->iscsi_ep);
> - }
> -
> - qedi_conn_destroy(qedi_conn->cls_conn);
> -
> - qedi_session_destroy(cls_sess);
> -}
> -
> void qedi_process_tcp_error(struct qedi_endpoint *ep,
> struct iscsi_eqe_data *data)
> {
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c index
> edf915432704..0b0acb827071 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -2417,11 +2417,9 @@ static void __qedi_remove(struct pci_dev *pdev, int
> mode)
> int rval;
> u16 retry = 10;
>
> - if (mode == QEDI_MODE_SHUTDOWN)
> - iscsi_host_for_each_session(qedi->shost,
> - qedi_clear_session_ctx);
> -
> if (mode == QEDI_MODE_NORMAL || mode ==
> QEDI_MODE_SHUTDOWN) {
> + iscsi_host_remove(qedi->shost);
> +
> if (qedi->tmf_thread) {
> flush_workqueue(qedi->tmf_thread);
> destroy_workqueue(qedi->tmf_thread);
> @@ -2482,7 +2480,6 @@ static void __qedi_remove(struct pci_dev *pdev, int
> mode)
> if (qedi->boot_kset)
> iscsi_boot_destroy_kset(qedi->boot_kset);
>
> - iscsi_host_remove(qedi->shost);
> iscsi_host_free(qedi->shost);
> }
> }
> --
> 2.25.1
Thanks,
Reviewed-by: Manish Rangankar <mrangankar@marvell.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] scsi: qedi: Fix host removal with running sessions
2021-06-09 19:27 [PATCH 1/1] scsi: qedi: Fix host removal with running sessions Mike Christie
2021-06-10 3:07 ` Martin K. Petersen
2021-06-10 4:44 ` [EXT] " Manish Rangankar
@ 2021-06-16 3:49 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2021-06-16 3:49 UTC (permalink / raw)
To: linux-scsi, mrangankar, Mike Christie; +Cc: Martin K . Petersen
On Wed, 9 Jun 2021 14:27:09 -0500, Mike Christie wrote:
> qedi_clear_session_ctx could race with the in-kernel or userspace driven
> recovery/removal and we could access a NULL conn or do a double free.
>
> We should be using iscsi_host_remove to start the removal process from the
> driver. It will start the in-kernel recovery and notify userspace that the
> driver's scsi_hosts are being removed. iscsid will then drive the session
> removal like is done when the logout command is run. When the sessions are
> removed, iscsi_host_remove will return so qedi can finish knowing there
> are no running sessions and no new sessions will be allowed.
>
> [...]
Applied to 5.14/scsi-queue, thanks!
[1/1] scsi: qedi: Fix host removal with running sessions
https://git.kernel.org/mkp/scsi/c/d1f2ce77638d
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-16 3:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 19:27 [PATCH 1/1] scsi: qedi: Fix host removal with running sessions Mike Christie
2021-06-10 3:07 ` Martin K. Petersen
2021-06-10 4:44 ` [EXT] " Manish Rangankar
2021-06-16 3:49 ` 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.