| Message ID | 20200718064921.9280-1-ap420073@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] netdevsim: fix unbalaced locking in nsim_create() | expand |
On Sat, 18 Jul 2020 06:49:21 +0000 Taehee Yoo wrote: > In the nsim_create(), rtnl_lock() is called before nsim_bpf_init(). > If nsim_bpf_init() is failed, rtnl_unlock() should be called, > but it isn't called. > So, unbalanced locking would occur. > > Fixes: e05b2d141fef ("netdevsim: move netdev creation/destruction to dev probe") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> > --- > drivers/net/netdevsim/netdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c > index 2908e0a0d6e1..b2a67a88b6ee 100644 > --- a/drivers/net/netdevsim/netdev.c > +++ b/drivers/net/netdevsim/netdev.c > @@ -316,8 +316,8 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) > err_ipsec_teardown: > nsim_ipsec_teardown(ns); > nsim_bpf_uninit(ns); > - rtnl_unlock(); > err_free_netdev: Could you rename this label err_unlock, since it's not pointing to free_netdev any more? > + rtnl_unlock(); > free_netdev(dev); > return ERR_PTR(err); > }
On Tue, 21 Jul 2020 at 02:57, Jakub Kicinski <kuba@kernel.org> wrote: > Hi Jakub, Thank you for your review! > On Sat, 18 Jul 2020 06:49:21 +0000 Taehee Yoo wrote: > > In the nsim_create(), rtnl_lock() is called before nsim_bpf_init(). > > If nsim_bpf_init() is failed, rtnl_unlock() should be called, > > but it isn't called. > > So, unbalanced locking would occur. > > > > Fixes: e05b2d141fef ("netdevsim: move netdev creation/destruction to dev probe") > > Signed-off-by: Taehee Yoo <ap420073@gmail.com> > > --- > > drivers/net/netdevsim/netdev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c > > index 2908e0a0d6e1..b2a67a88b6ee 100644 > > --- a/drivers/net/netdevsim/netdev.c > > +++ b/drivers/net/netdevsim/netdev.c > > @@ -316,8 +316,8 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) > > err_ipsec_teardown: > > nsim_ipsec_teardown(ns); > > nsim_bpf_uninit(ns); > > - rtnl_unlock(); > > err_free_netdev: > > Could you rename this label err_unlock, since it's not pointing to > free_netdev any more? > I will send a v2 patch to change the label name. Thanks a lot! Taehee Yoo
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index 2908e0a0d6e1..b2a67a88b6ee 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -316,8 +316,8 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) err_ipsec_teardown: nsim_ipsec_teardown(ns); nsim_bpf_uninit(ns); - rtnl_unlock(); err_free_netdev: + rtnl_unlock(); free_netdev(dev); return ERR_PTR(err); }
In the nsim_create(), rtnl_lock() is called before nsim_bpf_init(). If nsim_bpf_init() is failed, rtnl_unlock() should be called, but it isn't called. So, unbalanced locking would occur. Fixes: e05b2d141fef ("netdevsim: move netdev creation/destruction to dev probe") Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- drivers/net/netdevsim/netdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)