* [dm-level] master - libmultipath: fix use after free when iscsi logs in
@ 2020-07-09 7:08 lixiaokeng
0 siblings, 0 replies; 3+ messages in thread
From: lixiaokeng @ 2020-07-09 7:08 UTC (permalink / raw)
To: dm-devel; +Cc: linfeilong, liuzhiqiang26, lutianxiong
When two iscsi ips log in and out alternately and the following scripts run
at the same time,
#!/bin/bash
interval=5
while true
do
iscsiadm -m node -p 9.41.147.171 &> /dev/null
iscsiadm -m node -p 9.41.148.172 &> /dev/null
iscsiadm -m session &> /dev/null
rescan-scsi-bus.sh &> /dev/null
multipath -v2 &> /dev/null
multipath -ll &> /dev/null
sleep $interval
done
multipathd will have a segfault after about 30 mins.
The reason is that mpp->hwe is accessed after hwe is already freed. In
extract_hwe_from_path func, mpp->hwe is set to pp->hwe, so they points to the
same hwe. For some reasons, pp->mpp will be set to NULL in orphan_path func.
Then, pp and hwe will be freed with (pp->mpp == NULL) in free_path func
called by ev_remove_path func. However, mpp->hwe is not set to NULL while hwe
is already freed. So, when iscsi device logs in and new path is added to mpp,
mpp->hwe will be accessed in select_pgfailback func. Finally, use-after-free
problem occurs.
The procedure details given as follows,
1.wait_dmevents thread
wait_dmevents
->dmevent_loop
->update_multipath
->__setup_multipath
->update_multipath_strings
-> sync_paths
->orphan_path
2.uevqloop thread (iscsi log out, remove path)
uevqloop
->uevent_dispatch
->service_uevq
->uev_remove_path
->ev_remove_path //pp->mpp is NULL
->free_path(pp)
//pp->hew are freed but mpp->hwe is not set to NULL
3.ev_remove_path (iscsi log in, add path)
uevqloop
->uevent_dispatch
->service_uevq
->ev_add_path
->select_pgfailback //mpp->hwe is accessed
Here, we will set mpp->hwe to NULL before setting pp->map to NULL in orphan_path
func.
Signed-off-by: Tianxiong Lu <lutianxiong@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
libmultipath/structs_vec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 3dbbaa0..9bbe5d1 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -93,6 +93,8 @@ int adopt_paths(vector pathvec, struct multipath *mpp)
void orphan_path(struct path *pp, const char *reason)
{
condlog(3, "%s: orphan path, %s", pp->dev, reason);
+ if (pp->mpp && pp->mpp->hwe == pp->hwe)
+ pp->mpp->hwe = NULL;
pp->mpp = NULL;
pp->dmstate = PSTATE_UNDEF;
pp->uid_attribute = NULL;
--
2.26.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [dm-level] master - libmultipath: fix use after free when iscsi logs in
2020-07-13 2:22 lixiaokeng
@ 2020-07-13 10:49 ` Martin Wilck
0 siblings, 0 replies; 3+ messages in thread
From: Martin Wilck @ 2020-07-13 10:49 UTC (permalink / raw)
To: lixiaokeng, Christophe Varoqui
Cc: linfeilong, dm-devel, liuzhiqiang26, lutianxiong
Hello Lixiaokeng,
Thanks for the report and the analysis.
On Mon, 2020-07-13 at 10:22 +0800, lixiaokeng wrote:
> When two iscsi ips log in and out alternately and the following
> scripts run
> at the same time,
>
> #!/bin/bash
> interval=5
> while true
> do
> iscsiadm -m node -p 9.41.147.171 &> /dev/null
> iscsiadm -m node -p 9.41.148.172 &> /dev/null
> iscsiadm -m session &> /dev/null
> rescan-scsi-bus.sh &> /dev/null
> multipath -v2 &> /dev/null
> multipath -ll &> /dev/null
> sleep $interval
> done
>
> multipathd will have a segfault after about 30 mins.
>
> The reason is that mpp->hwe is accessed after hwe is already freed.
> In
> extract_hwe_from_path func, mpp->hwe is set to pp->hwe, so they
> points to the
> same hwe. For some reasons, pp->mpp will be set to NULL in
> orphan_path func.
> Then, pp and hwe will be freed with (pp->mpp == NULL) in free_path
> func
> called by ev_remove_path func. However, mpp->hwe is not set to NULL
> while hwe
> is already freed. So, when iscsi device logs in and new path is added
> to mpp,
> mpp->hwe will be accessed in select_pgfailback func. Finally, use-
> after-free
> problem occurs.
>
> The procedure details given as follows,
> 1.wait_dmevents thread
> wait_dmevents
> ->dmevent_loop
> ->update_multipath
> ->__setup_multipath
> ->update_multipath_strings
> -> sync_paths
> ->orphan_path
> 2.uevqloop thread (iscsi log out, remove path)
> uevqloop
> ->uevent_dispatch
> ->service_uevq
> ->uev_remove_path
> ->ev_remove_path //pp->mpp is NULL
> ->free_path(pp)
> //pp->hew are freed but mpp->hwe is not set to
> NULL
> 3.ev_remove_path (iscsi log in, add path)
> uevqloop
> ->uevent_dispatch
> ->service_uevq
> ->ev_add_path
> ->select_pgfailback //mpp->hwe is accessed
>
> Here, we will set mpp->hwe to NULL before setting pp->map to NULL in
> orphan_path
> func.
>
> Signed-off-by: Tianxiong Lu <lutianxiong@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> libmultipath/structs_vec.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index 3dbbaa0..9bbe5d1 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -93,6 +93,8 @@ int adopt_paths(vector pathvec, struct multipath
> *mpp)
> void orphan_path(struct path *pp, const char *reason)
> {
> condlog(3, "%s: orphan path, %s", pp->dev, reason);
> + if (pp->mpp && pp->mpp->hwe == pp->hwe)
> + pp->mpp->hwe = NULL;
> pp->mpp = NULL;
> pp->dmstate = PSTATE_UNDEF;
> pp->uid_attribute = NULL;
This isn't wrong, but I think sync_paths() has to be fixed as well.
I'll send a small series soon, based on your patch.
Martin
^ permalink raw reply [flat|nested] 3+ messages in thread
* [dm-level] master - libmultipath: fix use after free when iscsi logs in
@ 2020-07-13 2:22 lixiaokeng
2020-07-13 10:49 ` Martin Wilck
0 siblings, 1 reply; 3+ messages in thread
From: lixiaokeng @ 2020-07-13 2:22 UTC (permalink / raw)
To: Christophe Varoqui, Martin Wilck
Cc: linfeilong, dm-devel, liuzhiqiang26, lutianxiong
When two iscsi ips log in and out alternately and the following scripts run
at the same time,
#!/bin/bash
interval=5
while true
do
iscsiadm -m node -p 9.41.147.171 &> /dev/null
iscsiadm -m node -p 9.41.148.172 &> /dev/null
iscsiadm -m session &> /dev/null
rescan-scsi-bus.sh &> /dev/null
multipath -v2 &> /dev/null
multipath -ll &> /dev/null
sleep $interval
done
multipathd will have a segfault after about 30 mins.
The reason is that mpp->hwe is accessed after hwe is already freed. In
extract_hwe_from_path func, mpp->hwe is set to pp->hwe, so they points to the
same hwe. For some reasons, pp->mpp will be set to NULL in orphan_path func.
Then, pp and hwe will be freed with (pp->mpp == NULL) in free_path func
called by ev_remove_path func. However, mpp->hwe is not set to NULL while hwe
is already freed. So, when iscsi device logs in and new path is added to mpp,
mpp->hwe will be accessed in select_pgfailback func. Finally, use-after-free
problem occurs.
The procedure details given as follows,
1.wait_dmevents thread
wait_dmevents
->dmevent_loop
->update_multipath
->__setup_multipath
->update_multipath_strings
-> sync_paths
->orphan_path
2.uevqloop thread (iscsi log out, remove path)
uevqloop
->uevent_dispatch
->service_uevq
->uev_remove_path
->ev_remove_path //pp->mpp is NULL
->free_path(pp)
//pp->hew are freed but mpp->hwe is not set to NULL
3.ev_remove_path (iscsi log in, add path)
uevqloop
->uevent_dispatch
->service_uevq
->ev_add_path
->select_pgfailback //mpp->hwe is accessed
Here, we will set mpp->hwe to NULL before setting pp->map to NULL in orphan_path
func.
Signed-off-by: Tianxiong Lu <lutianxiong@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
libmultipath/structs_vec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 3dbbaa0..9bbe5d1 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -93,6 +93,8 @@ int adopt_paths(vector pathvec, struct multipath *mpp)
void orphan_path(struct path *pp, const char *reason)
{
condlog(3, "%s: orphan path, %s", pp->dev, reason);
+ if (pp->mpp && pp->mpp->hwe == pp->hwe)
+ pp->mpp->hwe = NULL;
pp->mpp = NULL;
pp->dmstate = PSTATE_UNDEF;
pp->uid_attribute = NULL;
--
2.26.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-13 10:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 7:08 [dm-level] master - libmultipath: fix use after free when iscsi logs in lixiaokeng
2020-07-13 2:22 lixiaokeng
2020-07-13 10:49 ` Martin Wilck
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.