| Message ID | 20210611015812.1626999-1-mudongliangabcd@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ieee802154: hwsim: Fix possible memory leak in hwsim_subscribe_all_others | expand |
Hi, On Thu, 10 Jun 2021 at 21:58, Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > In hwsim_subscribe_all_others, the error handling code performs > incorrectly if the second hwsim_alloc_edge fails. When this issue occurs, > it goes to sub_fail, without cleaning the edges allocated before. > > Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > drivers/net/ieee802154/mac802154_hwsim.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c > index da9135231c07..366eaae3550a 100644 > --- a/drivers/net/ieee802154/mac802154_hwsim.c > +++ b/drivers/net/ieee802154/mac802154_hwsim.c > @@ -715,6 +715,8 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy) > > return 0; > > +sub_fail: > + hwsim_edge_unsubscribe_me(phy); > me_fail: > rcu_read_lock(); > list_for_each_entry_rcu(e, &phy->edges, list) { > @@ -722,8 +724,6 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy) > hwsim_free_edge(e); > } > rcu_read_unlock(); > -sub_fail: this goto needs to be removed and all goto cases need to end in me_fail (better named to be fail only). In an error case both loops need to be iterated again to cleanup. - Alex
Hi, On Thu, 10 Jun 2021 at 21:58, Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > In hwsim_subscribe_all_others, the error handling code performs > incorrectly if the second hwsim_alloc_edge fails. When this issue occurs, > it goes to sub_fail, without cleaning the edges allocated before. > > Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> Acked-by: Alexander Aring <aahringo@redhat.com> sorry, it is a correct fix. Thanks. - Alex
Hello. On 12.06.21 14:49, Alexander Aring wrote: > Hi, > > On Thu, 10 Jun 2021 at 21:58, Dongliang Mu <mudongliangabcd@gmail.com> wrote: >> >> In hwsim_subscribe_all_others, the error handling code performs >> incorrectly if the second hwsim_alloc_edge fails. When this issue occurs, >> it goes to sub_fail, without cleaning the edges allocated before. >> >> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") >> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > Acked-by: Alexander Aring <aahringo@redhat.com> > > sorry, it is a correct fix. Thanks. This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index da9135231c07..366eaae3550a 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -715,6 +715,8 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy) return 0; +sub_fail: + hwsim_edge_unsubscribe_me(phy); me_fail: rcu_read_lock(); list_for_each_entry_rcu(e, &phy->edges, list) { @@ -722,8 +724,6 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy) hwsim_free_edge(e); } rcu_read_unlock(); -sub_fail: - hwsim_edge_unsubscribe_me(phy); return -ENOMEM; }
In hwsim_subscribe_all_others, the error handling code performs incorrectly if the second hwsim_alloc_edge fails. When this issue occurs, it goes to sub_fail, without cleaning the edges allocated before. Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb") Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> --- drivers/net/ieee802154/mac802154_hwsim.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)