* [PATCH] infiniband/core: Fix a use after free in cm_work_handler
@ 2021-03-11 2:21 Lv Yunlong
2021-03-11 9:22 ` Leon Romanovsky
0 siblings, 1 reply; 6+ messages in thread
From: Lv Yunlong @ 2021-03-11 2:21 UTC (permalink / raw)
To: dledford, jgg, linux-rdma; +Cc: linux-kernel, Lv Yunlong
In cm_work_handler, it calls destory_cm_id() to release
the initial reference of cm_id_priv taken by iw_create_cm_id()
and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id
(cm_id_priv) will be called and cause a use after free.
Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref")
Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn>
---
drivers/infiniband/core/iwcm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index da8adadf4755..cb6b4ac45e21 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work)
if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) {
ret = process_event(cm_id_priv, &levent);
- if (ret)
+ if (ret) {
destroy_cm_id(&cm_id_priv->id);
+ return;
+ }
} else
pr_debug("dropping event %d\n", levent.event);
if (iwcm_deref_id(cm_id_priv))
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler 2021-03-11 2:21 [PATCH] infiniband/core: Fix a use after free in cm_work_handler Lv Yunlong @ 2021-03-11 9:22 ` Leon Romanovsky 2021-03-11 10:29 ` lyl2019 0 siblings, 1 reply; 6+ messages in thread From: Leon Romanovsky @ 2021-03-11 9:22 UTC (permalink / raw) To: Lv Yunlong; +Cc: dledford, jgg, linux-rdma, linux-kernel On Wed, Mar 10, 2021 at 06:21:53PM -0800, Lv Yunlong wrote: > In cm_work_handler, it calls destory_cm_id() to release > the initial reference of cm_id_priv taken by iw_create_cm_id() > and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id > (cm_id_priv) will be called and cause a use after free. > > Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref") > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > --- > drivers/infiniband/core/iwcm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c > index da8adadf4755..cb6b4ac45e21 100644 > --- a/drivers/infiniband/core/iwcm.c > +++ b/drivers/infiniband/core/iwcm.c > @@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work) > > if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) { > ret = process_event(cm_id_priv, &levent); > - if (ret) > + if (ret) { > destroy_cm_id(&cm_id_priv->id); > + return; The destroy_cm_id() is called to free ->id and not cm_id_priv. This "return" leaks cm_id_priv now and what "a use after free" do you see? > + } > } else > pr_debug("dropping event %d\n", levent.event); > if (iwcm_deref_id(cm_id_priv)) > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler 2021-03-11 9:22 ` Leon Romanovsky @ 2021-03-11 10:29 ` lyl2019 2021-03-11 11:05 ` Leon Romanovsky 0 siblings, 1 reply; 6+ messages in thread From: lyl2019 @ 2021-03-11 10:29 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dledford, jgg, linux-rdma, linux-kernel In the implementation of destory_cm_id(), it restores cm_id_priv by "cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);". And the last line of destory_cm_id() calls "(void)iwcm_deref_id(cm_id_priv);" to free the cm_id_priv. > -----原始邮件----- > 发件人: "Leon Romanovsky" <leon@kernel.org> > 发送时间: 2021-03-11 17:22:03 (星期四) > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > 主题: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > On Wed, Mar 10, 2021 at 06:21:53PM -0800, Lv Yunlong wrote: > > In cm_work_handler, it calls destory_cm_id() to release > > the initial reference of cm_id_priv taken by iw_create_cm_id() > > and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id > > (cm_id_priv) will be called and cause a use after free. > > > > Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref") > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > --- > > drivers/infiniband/core/iwcm.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c > > index da8adadf4755..cb6b4ac45e21 100644 > > --- a/drivers/infiniband/core/iwcm.c > > +++ b/drivers/infiniband/core/iwcm.c > > @@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work) > > > > if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) { > > ret = process_event(cm_id_priv, &levent); > > - if (ret) > > + if (ret) { > > destroy_cm_id(&cm_id_priv->id); > > + return; > > The destroy_cm_id() is called to free ->id and not cm_id_priv. This "return" > leaks cm_id_priv now and what "a use after free" do you see? > > > + } > > } else > > pr_debug("dropping event %d\n", levent.event); > > if (iwcm_deref_id(cm_id_priv)) > > -- > > 2.25.1 > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler 2021-03-11 10:29 ` lyl2019 @ 2021-03-11 11:05 ` Leon Romanovsky 2021-03-11 12:03 ` lyl2019 0 siblings, 1 reply; 6+ messages in thread From: Leon Romanovsky @ 2021-03-11 11:05 UTC (permalink / raw) To: lyl2019; +Cc: dledford, jgg, linux-rdma, linux-kernel On Thu, Mar 11, 2021 at 06:29:19PM +0800, lyl2019@mail.ustc.edu.cn wrote: > In the implementation of destory_cm_id(), it restores cm_id_priv by > "cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);". > > And the last line of destory_cm_id() calls "(void)iwcm_deref_id(cm_id_priv);" > to free the cm_id_priv. It is not enough to see double call to iwcm_deref_id() because it is protected with refcount to claim use-after-free. Did you hit the BUG_ON() for the second call to iwcm_deref_id()? And please don't do top-posting. Thanks > > > > -----原始邮件----- > > 发件人: "Leon Romanovsky" <leon@kernel.org> > > 发送时间: 2021-03-11 17:22:03 (星期四) > > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > > 主题: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > > > On Wed, Mar 10, 2021 at 06:21:53PM -0800, Lv Yunlong wrote: > > > In cm_work_handler, it calls destory_cm_id() to release > > > the initial reference of cm_id_priv taken by iw_create_cm_id() > > > and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id > > > (cm_id_priv) will be called and cause a use after free. > > > > > > Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref") > > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > > --- > > > drivers/infiniband/core/iwcm.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c > > > index da8adadf4755..cb6b4ac45e21 100644 > > > --- a/drivers/infiniband/core/iwcm.c > > > +++ b/drivers/infiniband/core/iwcm.c > > > @@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work) > > > > > > if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) { > > > ret = process_event(cm_id_priv, &levent); > > > - if (ret) > > > + if (ret) { > > > destroy_cm_id(&cm_id_priv->id); > > > + return; > > > > The destroy_cm_id() is called to free ->id and not cm_id_priv. This "return" > > leaks cm_id_priv now and what "a use after free" do you see? > > > > > + } > > > } else > > > pr_debug("dropping event %d\n", levent.event); > > > if (iwcm_deref_id(cm_id_priv)) > > > -- > > > 2.25.1 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler 2021-03-11 11:05 ` Leon Romanovsky @ 2021-03-11 12:03 ` lyl2019 2021-03-11 14:12 ` Leon Romanovsky 0 siblings, 1 reply; 6+ messages in thread From: lyl2019 @ 2021-03-11 12:03 UTC (permalink / raw) To: Leon Romanovsky; +Cc: dledford, jgg, linux-rdma, linux-kernel > -----原始邮件----- > 发件人: "Leon Romanovsky" <leon@kernel.org> > 发送时间: 2021-03-11 19:05:03 (星期四) > 收件人: lyl2019@mail.ustc.edu.cn > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > 主题: Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > On Thu, Mar 11, 2021 at 06:29:19PM +0800, lyl2019@mail.ustc.edu.cn wrote: > > In the implementation of destory_cm_id(), it restores cm_id_priv by > > "cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);". > > > > And the last line of destory_cm_id() calls "(void)iwcm_deref_id(cm_id_priv);" > > to free the cm_id_priv. > > It is not enough to see double call to iwcm_deref_id() because it is > protected with refcount to claim use-after-free. Did you hit the BUG_ON() > for the second call to iwcm_deref_id()? > > And please don't do top-posting. > > Thanks > > > > > > > > -----原始邮件----- > > > 发件人: "Leon Romanovsky" <leon@kernel.org> > > > 发送时间: 2021-03-11 17:22:03 (星期四) > > > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > > > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > > > 主题: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > > > > > On Wed, Mar 10, 2021 at 06:21:53PM -0800, Lv Yunlong wrote: > > > > In cm_work_handler, it calls destory_cm_id() to release > > > > the initial reference of cm_id_priv taken by iw_create_cm_id() > > > > and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id > > > > (cm_id_priv) will be called and cause a use after free. > > > > > > > > Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref") > > > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > > > --- > > > > drivers/infiniband/core/iwcm.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c > > > > index da8adadf4755..cb6b4ac45e21 100644 > > > > --- a/drivers/infiniband/core/iwcm.c > > > > +++ b/drivers/infiniband/core/iwcm.c > > > > @@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work) > > > > > > > > if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) { > > > > ret = process_event(cm_id_priv, &levent); > > > > - if (ret) > > > > + if (ret) { > > > > destroy_cm_id(&cm_id_priv->id); > > > > + return; > > > > > > The destroy_cm_id() is called to free ->id and not cm_id_priv. This "return" > > > leaks cm_id_priv now and what "a use after free" do you see? > > > > > > > + } > > > > } else > > > > pr_debug("dropping event %d\n", levent.event); > > > > if (iwcm_deref_id(cm_id_priv)) > > > > -- > > > > 2.25.1 > > > > > > > > I'm not familiar with debug the kernel, sorry.This problem was reported by my code analyzer and reviewed by myself. But i think as long as destroy_cm_id() is called, iwcm_deref_id() will be called twice. Then "BUG_ON(atomic_read(&cm_id_priv->refcount)==0);" in iwcm_deref_id() will be triggered. Is it not a true problem? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler 2021-03-11 12:03 ` lyl2019 @ 2021-03-11 14:12 ` Leon Romanovsky 0 siblings, 0 replies; 6+ messages in thread From: Leon Romanovsky @ 2021-03-11 14:12 UTC (permalink / raw) To: lyl2019; +Cc: dledford, jgg, linux-rdma, linux-kernel On Thu, Mar 11, 2021 at 08:03:23PM +0800, lyl2019@mail.ustc.edu.cn wrote: > > > > > -----原始邮件----- > > 发件人: "Leon Romanovsky" <leon@kernel.org> > > 发送时间: 2021-03-11 19:05:03 (星期四) > > 收件人: lyl2019@mail.ustc.edu.cn > > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > > 主题: Re: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > > > On Thu, Mar 11, 2021 at 06:29:19PM +0800, lyl2019@mail.ustc.edu.cn wrote: > > > In the implementation of destory_cm_id(), it restores cm_id_priv by > > > "cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);". > > > > > > And the last line of destory_cm_id() calls "(void)iwcm_deref_id(cm_id_priv);" > > > to free the cm_id_priv. > > > > It is not enough to see double call to iwcm_deref_id() because it is > > protected with refcount to claim use-after-free. Did you hit the BUG_ON() > > for the second call to iwcm_deref_id()? > > > > And please don't do top-posting. > > > > Thanks > > > > > > > > > > > > -----原始邮件----- > > > > 发件人: "Leon Romanovsky" <leon@kernel.org> > > > > 发送时间: 2021-03-11 17:22:03 (星期四) > > > > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > > > > 抄送: dledford@redhat.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > > > > 主题: Re: [PATCH] infiniband/core: Fix a use after free in cm_work_handler > > > > > > > > On Wed, Mar 10, 2021 at 06:21:53PM -0800, Lv Yunlong wrote: > > > > > In cm_work_handler, it calls destory_cm_id() to release > > > > > the initial reference of cm_id_priv taken by iw_create_cm_id() > > > > > and free the cm_id_priv. After destory_cm_id(), iwcm_deref_id > > > > > (cm_id_priv) will be called and cause a use after free. > > > > > > > > > > Fixes: 59c68ac31e15a ("iw_cm: free cm_id resources on the last deref") > > > > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > > > > --- > > > > > drivers/infiniband/core/iwcm.c | 4 +++- > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c > > > > > index da8adadf4755..cb6b4ac45e21 100644 > > > > > --- a/drivers/infiniband/core/iwcm.c > > > > > +++ b/drivers/infiniband/core/iwcm.c > > > > > @@ -1035,8 +1035,10 @@ static void cm_work_handler(struct work_struct *_work) > > > > > > > > > > if (!test_bit(IWCM_F_DROP_EVENTS, &cm_id_priv->flags)) { > > > > > ret = process_event(cm_id_priv, &levent); > > > > > - if (ret) > > > > > + if (ret) { > > > > > destroy_cm_id(&cm_id_priv->id); > > > > > + return; > > > > > > > > The destroy_cm_id() is called to free ->id and not cm_id_priv. This "return" > > > > leaks cm_id_priv now and what "a use after free" do you see? > > > > > > > > > + } > > > > > } else > > > > > pr_debug("dropping event %d\n", levent.event); > > > > > if (iwcm_deref_id(cm_id_priv)) > > > > > -- > > > > > 2.25.1 > > > > > > > > > > > > I'm not familiar with debug the kernel, sorry.This problem was > reported by my code analyzer and reviewed by myself. > > But i think as long as destroy_cm_id() is called, iwcm_deref_id() will be called twice. > Then "BUG_ON(atomic_read(&cm_id_priv->refcount)==0);" in iwcm_deref_id() will be triggered. > > Is it not a true problem? I don't know, this is I'm trying to understand here. Thanks ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-11 14:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 2:21 [PATCH] infiniband/core: Fix a use after free in cm_work_handler Lv Yunlong 2021-03-11 9:22 ` Leon Romanovsky 2021-03-11 10:29 ` lyl2019 2021-03-11 11:05 ` Leon Romanovsky 2021-03-11 12:03 ` lyl2019 2021-03-11 14:12 ` Leon Romanovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).