* [PATCH rdma-next] IB/iser: Fix connection teardown race condition
@ 2017-05-21 16:17 ` Leon Romanovsky
0 siblings, 0 replies; 7+ messages in thread
From: Leon Romanovsky @ 2017-05-21 16:17 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Vladimir Neyelov,
stable-u79uwXL29TY76Z2rM5mHXA
From: Vladimir Neyelov <vladimirn-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Under heavy iser target(scst) start/stop stress during login/logout
on iser intitiator side happened trace call provided below.
The function iscsi_iser_slave_alloc iser_conn pointer could be NULL,
due to the fact that function iscsi_iser_conn_stop can be called before
and free iser connection. Let's protect that flow by introducing global mutex.
BUG: unable to handle kernel paging request at 0000000000001018
IP: [<ffffffffc0426f7e>] iscsi_iser_slave_alloc+0x1e/0x50 [ib_iser]
Call Trace:
? scsi_alloc_sdev+0x242/0x300
scsi_probe_and_add_lun+0x9e1/0xea0
? kfree_const+0x21/0x30
? kobject_set_name_vargs+0x76/0x90
? __pm_runtime_resume+0x5b/0x70
__scsi_scan_target+0xf6/0x250
scsi_scan_target+0xea/0x100
iscsi_user_scan_session.part.13+0x101/0x130 [scsi_transport_iscsi]
? iscsi_user_scan_session.part.13+0x130/0x130 [scsi_transport_iscsi]
iscsi_user_scan_session+0x1e/0x30 [scsi_transport_iscsi]
device_for_each_child+0x50/0x90
iscsi_user_scan+0x44/0x60 [scsi_transport_iscsi]
store_scan+0xa8/0x100
? common_file_perm+0x5d/0x1c0
dev_attr_store+0x18/0x30
sysfs_kf_write+0x37/0x40
kernfs_fop_write+0x12c/0x1c0
__vfs_write+0x18/0x40
vfs_write+0xb5/0x1a0
SyS_write+0x55/0xc0
Fixes: 318d311e8f01 ("iser: Accept arbitrary sg lists mapping if the device supports it")
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.5+
Signed-off-by: Vladimir Neyelov <vladimirn-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 5a887efb4bdf..9ba649836d86 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -83,6 +83,7 @@ static struct scsi_host_template iscsi_iser_sht;
static struct iscsi_transport iscsi_iser_transport;
static struct scsi_transport_template *iscsi_iser_scsi_transport;
static struct workqueue_struct *release_wq;
+static DEFINE_MUTEX(unbind_iser_conn_mutex);
struct iser_global ig;
int iser_debug_level = 0;
@@ -550,12 +551,14 @@ iscsi_iser_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
*/
if (iser_conn) {
mutex_lock(&iser_conn->state_mutex);
+ mutex_lock(&unbind_iser_conn_mutex);
iser_conn_terminate(iser_conn);
iscsi_conn_stop(cls_conn, flag);
/* unbind */
iser_conn->iscsi_conn = NULL;
conn->dd_data = NULL;
+ mutex_unlock(&unbind_iser_conn_mutex);
complete(&iser_conn->stop_completion);
mutex_unlock(&iser_conn->state_mutex);
@@ -977,13 +980,21 @@ static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
struct iser_conn *iser_conn;
struct ib_device *ib_dev;
+ mutex_lock(&unbind_iser_conn_mutex);
+
session = starget_to_session(scsi_target(sdev))->dd_data;
iser_conn = session->leadconn->dd_data;
+ if (!iser_conn) {
+ mutex_unlock(&unbind_iser_conn_mutex);
+ return -ENOTCONN;
+ }
ib_dev = iser_conn->ib_conn.device->ib_device;
if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
+ mutex_unlock(&unbind_iser_conn_mutex);
+
return 0;
}
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH rdma-next] IB/iser: Fix connection teardown race condition
@ 2017-05-21 16:17 ` Leon Romanovsky
0 siblings, 0 replies; 7+ messages in thread
From: Leon Romanovsky @ 2017-05-21 16:17 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-rdma, Vladimir Neyelov, stable
From: Vladimir Neyelov <vladimirn@mellanox.com>
Under heavy iser target(scst) start/stop stress during login/logout
on iser intitiator side happened trace call provided below.
The function iscsi_iser_slave_alloc iser_conn pointer could be NULL,
due to the fact that function iscsi_iser_conn_stop can be called before
and free iser connection. Let's protect that flow by introducing global mutex.
BUG: unable to handle kernel paging request at 0000000000001018
IP: [<ffffffffc0426f7e>] iscsi_iser_slave_alloc+0x1e/0x50 [ib_iser]
Call Trace:
? scsi_alloc_sdev+0x242/0x300
scsi_probe_and_add_lun+0x9e1/0xea0
? kfree_const+0x21/0x30
? kobject_set_name_vargs+0x76/0x90
? __pm_runtime_resume+0x5b/0x70
__scsi_scan_target+0xf6/0x250
scsi_scan_target+0xea/0x100
iscsi_user_scan_session.part.13+0x101/0x130 [scsi_transport_iscsi]
? iscsi_user_scan_session.part.13+0x130/0x130 [scsi_transport_iscsi]
iscsi_user_scan_session+0x1e/0x30 [scsi_transport_iscsi]
device_for_each_child+0x50/0x90
iscsi_user_scan+0x44/0x60 [scsi_transport_iscsi]
store_scan+0xa8/0x100
? common_file_perm+0x5d/0x1c0
dev_attr_store+0x18/0x30
sysfs_kf_write+0x37/0x40
kernfs_fop_write+0x12c/0x1c0
__vfs_write+0x18/0x40
vfs_write+0xb5/0x1a0
SyS_write+0x55/0xc0
Fixes: 318d311e8f01 ("iser: Accept arbitrary sg lists mapping if the device supports it")
Cc: <stable@vger.kernel.org> # v4.5+
Signed-off-by: Vladimir Neyelov <vladimirn@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 5a887efb4bdf..9ba649836d86 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -83,6 +83,7 @@ static struct scsi_host_template iscsi_iser_sht;
static struct iscsi_transport iscsi_iser_transport;
static struct scsi_transport_template *iscsi_iser_scsi_transport;
static struct workqueue_struct *release_wq;
+static DEFINE_MUTEX(unbind_iser_conn_mutex);
struct iser_global ig;
int iser_debug_level = 0;
@@ -550,12 +551,14 @@ iscsi_iser_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
*/
if (iser_conn) {
mutex_lock(&iser_conn->state_mutex);
+ mutex_lock(&unbind_iser_conn_mutex);
iser_conn_terminate(iser_conn);
iscsi_conn_stop(cls_conn, flag);
/* unbind */
iser_conn->iscsi_conn = NULL;
conn->dd_data = NULL;
+ mutex_unlock(&unbind_iser_conn_mutex);
complete(&iser_conn->stop_completion);
mutex_unlock(&iser_conn->state_mutex);
@@ -977,13 +980,21 @@ static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
struct iser_conn *iser_conn;
struct ib_device *ib_dev;
+ mutex_lock(&unbind_iser_conn_mutex);
+
session = starget_to_session(scsi_target(sdev))->dd_data;
iser_conn = session->leadconn->dd_data;
+ if (!iser_conn) {
+ mutex_unlock(&unbind_iser_conn_mutex);
+ return -ENOTCONN;
+ }
ib_dev = iser_conn->ib_conn.device->ib_device;
if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
+ mutex_unlock(&unbind_iser_conn_mutex);
+
return 0;
}
--
2.12.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH rdma-next] IB/iser: Fix connection teardown race condition
2017-05-21 16:17 ` Leon Romanovsky
@ 2017-05-25 15:24 ` Max Gurtovoy
-1 siblings, 0 replies; 7+ messages in thread
From: Max Gurtovoy @ 2017-05-25 15:24 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford; +Cc: linux-rdma, Vladimir Neyelov, stable, sagig
Hi Sagi,
what do you think on this approach ?
we must somehow defend on the NULL deref in case dd_data in iscsi_conn
(the iser_conn) already NULL due to conn_stop call.
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 5a887efb4bdf..9ba649836d86 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -83,6 +83,7 @@ static struct scsi_host_template iscsi_iser_sht;
> static struct iscsi_transport iscsi_iser_transport;
> static struct scsi_transport_template *iscsi_iser_scsi_transport;
> static struct workqueue_struct *release_wq;
> +static DEFINE_MUTEX(unbind_iser_conn_mutex);
> struct iser_global ig;
>
> int iser_debug_level = 0;
> @@ -550,12 +551,14 @@ iscsi_iser_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
> */
> if (iser_conn) {
> mutex_lock(&iser_conn->state_mutex);
> + mutex_lock(&unbind_iser_conn_mutex);
> iser_conn_terminate(iser_conn);
> iscsi_conn_stop(cls_conn, flag);
>
> /* unbind */
> iser_conn->iscsi_conn = NULL;
> conn->dd_data = NULL;
> + mutex_unlock(&unbind_iser_conn_mutex);
>
> complete(&iser_conn->stop_completion);
> mutex_unlock(&iser_conn->state_mutex);
> @@ -977,13 +980,21 @@ static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
> struct iser_conn *iser_conn;
> struct ib_device *ib_dev;
>
> + mutex_lock(&unbind_iser_conn_mutex);
> +
> session = starget_to_session(scsi_target(sdev))->dd_data;
> iser_conn = session->leadconn->dd_data;
> + if (!iser_conn) {
> + mutex_unlock(&unbind_iser_conn_mutex);
> + return -ENOTCONN;
> + }
> ib_dev = iser_conn->ib_conn.device->ib_device;
>
> if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
> blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
>
> + mutex_unlock(&unbind_iser_conn_mutex);
> +
> return 0;
> }
>
> --
> 2.12.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH rdma-next] IB/iser: Fix connection teardown race condition
@ 2017-05-25 15:24 ` Max Gurtovoy
0 siblings, 0 replies; 7+ messages in thread
From: Max Gurtovoy @ 2017-05-25 15:24 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford; +Cc: linux-rdma, Vladimir Neyelov, stable, sagig
Hi Sagi,
what do you think on this approach ?
we must somehow defend on the NULL deref in case dd_data in iscsi_conn
(the iser_conn) already NULL due to conn_stop call.
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 5a887efb4bdf..9ba649836d86 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -83,6 +83,7 @@ static struct scsi_host_template iscsi_iser_sht;
> static struct iscsi_transport iscsi_iser_transport;
> static struct scsi_transport_template *iscsi_iser_scsi_transport;
> static struct workqueue_struct *release_wq;
> +static DEFINE_MUTEX(unbind_iser_conn_mutex);
> struct iser_global ig;
>
> int iser_debug_level = 0;
> @@ -550,12 +551,14 @@ iscsi_iser_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
> */
> if (iser_conn) {
> mutex_lock(&iser_conn->state_mutex);
> + mutex_lock(&unbind_iser_conn_mutex);
> iser_conn_terminate(iser_conn);
> iscsi_conn_stop(cls_conn, flag);
>
> /* unbind */
> iser_conn->iscsi_conn = NULL;
> conn->dd_data = NULL;
> + mutex_unlock(&unbind_iser_conn_mutex);
>
> complete(&iser_conn->stop_completion);
> mutex_unlock(&iser_conn->state_mutex);
> @@ -977,13 +980,21 @@ static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
> struct iser_conn *iser_conn;
> struct ib_device *ib_dev;
>
> + mutex_lock(&unbind_iser_conn_mutex);
> +
> session = starget_to_session(scsi_target(sdev))->dd_data;
> iser_conn = session->leadconn->dd_data;
> + if (!iser_conn) {
> + mutex_unlock(&unbind_iser_conn_mutex);
> + return -ENOTCONN;
> + }
> ib_dev = iser_conn->ib_conn.device->ib_device;
>
> if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
> blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
>
> + mutex_unlock(&unbind_iser_conn_mutex);
> +
> return 0;
> }
>
> --
> 2.12.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH rdma-next] IB/iser: Fix connection teardown race condition
2017-05-25 15:24 ` Max Gurtovoy
(?)
@ 2017-05-30 11:50 ` Sagi Grimberg
[not found] ` <fa976fe5-be25-1c6a-5997-8fa7524e89d8-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
-1 siblings, 1 reply; 7+ messages in thread
From: Sagi Grimberg @ 2017-05-30 11:50 UTC (permalink / raw)
To: Max Gurtovoy, Leon Romanovsky, Doug Ledford
Cc: linux-rdma, Vladimir Neyelov, stable
> Hi Sagi,
>
> what do you think on this approach ?
> we must somehow defend on the NULL deref in case dd_data in iscsi_conn
> (the iser_conn) already NULL due to conn_stop call.
Sounds like we get conn_stop duing lun scanning. Generally speaking I
don't like a global lock for this, but given the non trivial iscsi state
machine
we can do with that.
Reviewed-by: Sagi Grimberg <sagi@grimbeg.me>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH rdma-next] IB/iser: Fix connection teardown race condition
2017-05-30 11:50 ` Sagi Grimberg
@ 2017-07-22 17:17 ` Doug Ledford
0 siblings, 0 replies; 7+ messages in thread
From: Doug Ledford @ 2017-07-22 17:17 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Leon Romanovsky
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Vladimir Neyelov,
stable-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]
On 5/30/2017 7:50 AM, Sagi Grimberg wrote:
>
>> Hi Sagi,
>>
>> what do you think on this approach ?
>> we must somehow defend on the NULL deref in case dd_data in iscsi_conn
>> (the iser_conn) already NULL due to conn_stop call.
>
> Sounds like we get conn_stop duing lun scanning. Generally speaking I
> don't like a global lock for this, but given the non trivial iscsi state
> machine
> we can do with that.
>
> Reviewed-by: Sagi Grimberg <sagi-egDjqUIXVlxBDLzU/O5InQ@public.gmane.org>
This patch was accepted into 4.13-rc, thanks.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH rdma-next] IB/iser: Fix connection teardown race condition
@ 2017-07-22 17:17 ` Doug Ledford
0 siblings, 0 replies; 7+ messages in thread
From: Doug Ledford @ 2017-07-22 17:17 UTC (permalink / raw)
To: Sagi Grimberg, Max Gurtovoy, Leon Romanovsky
Cc: linux-rdma, Vladimir Neyelov, stable
[-- Attachment #1.1: Type: text/plain, Size: 679 bytes --]
On 5/30/2017 7:50 AM, Sagi Grimberg wrote:
>
>> Hi Sagi,
>>
>> what do you think on this approach ?
>> we must somehow defend on the NULL deref in case dd_data in iscsi_conn
>> (the iser_conn) already NULL due to conn_stop call.
>
> Sounds like we get conn_stop duing lun scanning. Generally speaking I
> don't like a global lock for this, but given the non trivial iscsi state
> machine
> we can do with that.
>
> Reviewed-by: Sagi Grimberg <sagi@grimbeg.me>
This patch was accepted into 4.13-rc, thanks.
--
Doug Ledford <dledford@redhat.com>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-22 17:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-21 16:17 [PATCH rdma-next] IB/iser: Fix connection teardown race condition Leon Romanovsky
2017-05-21 16:17 ` Leon Romanovsky
2017-05-25 15:24 ` Max Gurtovoy
2017-05-25 15:24 ` Max Gurtovoy
2017-05-30 11:50 ` Sagi Grimberg
[not found] ` <fa976fe5-be25-1c6a-5997-8fa7524e89d8-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
2017-07-22 17:17 ` Doug Ledford
2017-07-22 17:17 ` Doug Ledford
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.