* [PATCH net] net: fix sysfs symlinks of adjacent devices @ 2014-09-12 10:13 Alexander Fomichev 2014-09-12 13:33 ` Vlad Yasevich 2014-09-19 8:59 ` [PATCH net] net: fix sysfs symlinks of adjacent devices Andres Freund 0 siblings, 2 replies; 7+ messages in thread From: Alexander Fomichev @ 2014-09-12 10:13 UTC (permalink / raw) To: netdev Cc: David Miller, Cong Wang, Vlad Yasevich, Andres Freund, Alexander Y. Fomichev From: "Alexander Y. Fomichev" <git.user@gmail.com> __netdev_adjacent_dev_insert may add adjacent device from another namespace. Without proper check it leads to emergence of broken symlink from/to device not existing in current namespace. Fix: check net_ns is the same before netdev_adjacent_sysfs_add/del related to: 4c75431ac3520631f1d9e74aa88407e6374dbbc4 Signed-off-by: Alexander Y. Fomichev <git.user@gmail.com> --- net/core/dev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index ab9a16530c36..887784b2dcde 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4841,7 +4841,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, pr_debug("dev_hold for %s, because of link added from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); - if (netdev_adjacent_is_neigh_list(dev, dev_list)) { + if (netdev_adjacent_is_neigh_list(dev, dev_list) && + net_eq(dev_net(dev), dev_net(adj_dev))) { ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list); if (ret) goto free_adj; @@ -4862,7 +4863,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, return 0; remove_symlinks: - if (netdev_adjacent_is_neigh_list(dev, dev_list)) + if (netdev_adjacent_is_neigh_list(dev, dev_list) && + net_eq(dev_net(dev), dev_net(adj_dev))) netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); free_adj: kfree(adj); -- 2.1.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fix sysfs symlinks of adjacent devices 2014-09-12 10:13 [PATCH net] net: fix sysfs symlinks of adjacent devices Alexander Fomichev @ 2014-09-12 13:33 ` Vlad Yasevich 2014-09-14 21:45 ` David Miller 2014-09-19 8:59 ` [PATCH net] net: fix sysfs symlinks of adjacent devices Andres Freund 1 sibling, 1 reply; 7+ messages in thread From: Vlad Yasevich @ 2014-09-12 13:33 UTC (permalink / raw) To: Alexander Fomichev, netdev Cc: David Miller, Cong Wang, Vlad Yasevich, Andres Freund On 09/12/2014 06:13 AM, Alexander Fomichev wrote: > From: "Alexander Y. Fomichev" <git.user@gmail.com> > > __netdev_adjacent_dev_insert may add adjacent device from another > namespace. Without proper check it leads to emergence of broken > symlink from/to device not existing in current namespace. > Fix: check net_ns is the same before netdev_adjacent_sysfs_add/del > related to: 4c75431ac3520631f1d9e74aa88407e6374dbbc4 > > Signed-off-by: Alexander Y. Fomichev <git.user@gmail.com> > --- > net/core/dev.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index ab9a16530c36..887784b2dcde 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4841,7 +4841,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, > pr_debug("dev_hold for %s, because of link added from %s to %s\n", > adj_dev->name, dev->name, adj_dev->name); > > - if (netdev_adjacent_is_neigh_list(dev, dev_list)) { > + if (netdev_adjacent_is_neigh_list(dev, dev_list) && > + net_eq(dev_net(dev), dev_net(adj_dev))) { > ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list); > if (ret) > goto free_adj; > @@ -4862,7 +4863,8 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, > return 0; > > remove_symlinks: > - if (netdev_adjacent_is_neigh_list(dev, dev_list)) > + if (netdev_adjacent_is_neigh_list(dev, dev_list) && > + net_eq(dev_net(dev), dev_net(adj_dev))) > netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); > free_adj: > kfree(adj); > Looking over the code, it might make sense to move all the net_eq checks into adjacent_sysfs calls so as to consolidate them. I haven't audited all code paths, but at first glance it should do the right thing. What do you think? -vlad ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fix sysfs symlinks of adjacent devices 2014-09-12 13:33 ` Vlad Yasevich @ 2014-09-14 21:45 ` David Miller 2014-09-15 10:18 ` Alexander Y. Fomichev 2014-09-15 10:22 ` [PATCH net] net: fix creation adjacent device symlinks Alexander Fomichev 0 siblings, 2 replies; 7+ messages in thread From: David Miller @ 2014-09-14 21:45 UTC (permalink / raw) To: vyasevich; +Cc: git.user, netdev, cwang, vyasevic, andres From: Vlad Yasevich <vyasevich@gmail.com> Date: Fri, 12 Sep 2014 09:33:39 -0400 > Looking over the code, it might make sense to move all the net_eq checks > into adjacent_sysfs calls so as to consolidate them. I haven't audited > all code paths, but at first glance it should do the right thing. > > What do you think? Agreed, let's do the following then? diff --git a/net/core/dev.c b/net/core/dev.c index ab9a165..a70e49e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4809,9 +4809,14 @@ static void netdev_adjacent_sysfs_del(struct net_device *dev, sysfs_remove_link(&(dev->dev.kobj), linkname); } -#define netdev_adjacent_is_neigh_list(dev, dev_list) \ - (dev_list == &dev->adj_list.upper || \ - dev_list == &dev->adj_list.lower) +static bool netdev_adjacent_is_neigh_list(struct net_device *dev, + struct net_device *adj_dev, + struct list_head *dev_list) +{ + return (dev_list == &dev->adj_list.upper || + dev_list == &dev->adj_list.lower) && + net_eq(dev_net(dev), dev_net(adj_dev)); +} static int __netdev_adjacent_dev_insert(struct net_device *dev, struct net_device *adj_dev, @@ -4841,7 +4846,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, pr_debug("dev_hold for %s, because of link added from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); - if (netdev_adjacent_is_neigh_list(dev, dev_list)) { + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) { ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list); if (ret) goto free_adj; @@ -4862,7 +4867,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, return 0; remove_symlinks: - if (netdev_adjacent_is_neigh_list(dev, dev_list)) + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); free_adj: kfree(adj); @@ -4895,8 +4900,7 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev, if (adj->master) sysfs_remove_link(&(dev->dev.kobj), "master"); - if (netdev_adjacent_is_neigh_list(dev, dev_list) && - net_eq(dev_net(dev),dev_net(adj_dev))) + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); list_del_rcu(&adj->list); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fix sysfs symlinks of adjacent devices 2014-09-14 21:45 ` David Miller @ 2014-09-15 10:18 ` Alexander Y. Fomichev 2014-09-15 10:22 ` [PATCH net] net: fix creation adjacent device symlinks Alexander Fomichev 1 sibling, 0 replies; 7+ messages in thread From: Alexander Y. Fomichev @ 2014-09-15 10:18 UTC (permalink / raw) To: David Miller; +Cc: vyasevich, netdev, Cong Wang, vyasevic, Andres Freund tnx, i had thinking move it to netdev_adjacent_sysfs_add/del.. but this is better. On Mon, Sep 15, 2014 at 1:45 AM, David Miller <davem@davemloft.net> wrote: > From: Vlad Yasevich <vyasevich@gmail.com> > Date: Fri, 12 Sep 2014 09:33:39 -0400 > >> Looking over the code, it might make sense to move all the net_eq checks >> into adjacent_sysfs calls so as to consolidate them. I haven't audited >> all code paths, but at first glance it should do the right thing. >> >> What do you think? > > Agreed, let's do the following then? > > diff --git a/net/core/dev.c b/net/core/dev.c > index ab9a165..a70e49e 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4809,9 +4809,14 @@ static void netdev_adjacent_sysfs_del(struct net_device *dev, > sysfs_remove_link(&(dev->dev.kobj), linkname); > } > > -#define netdev_adjacent_is_neigh_list(dev, dev_list) \ > - (dev_list == &dev->adj_list.upper || \ > - dev_list == &dev->adj_list.lower) > +static bool netdev_adjacent_is_neigh_list(struct net_device *dev, > + struct net_device *adj_dev, > + struct list_head *dev_list) > +{ > + return (dev_list == &dev->adj_list.upper || > + dev_list == &dev->adj_list.lower) && > + net_eq(dev_net(dev), dev_net(adj_dev)); > +} > > static int __netdev_adjacent_dev_insert(struct net_device *dev, > struct net_device *adj_dev, > @@ -4841,7 +4846,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, > pr_debug("dev_hold for %s, because of link added from %s to %s\n", > adj_dev->name, dev->name, adj_dev->name); > > - if (netdev_adjacent_is_neigh_list(dev, dev_list)) { > + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) { > ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list); > if (ret) > goto free_adj; > @@ -4862,7 +4867,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, > return 0; > > remove_symlinks: > - if (netdev_adjacent_is_neigh_list(dev, dev_list)) > + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) > netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); > free_adj: > kfree(adj); > @@ -4895,8 +4900,7 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev, > if (adj->master) > sysfs_remove_link(&(dev->dev.kobj), "master"); > > - if (netdev_adjacent_is_neigh_list(dev, dev_list) && > - net_eq(dev_net(dev),dev_net(adj_dev))) > + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) > netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); > > list_del_rcu(&adj->list); -- Best regards. Alexander Y. Fomichev <git.user@gmail.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net] net: fix creation adjacent device symlinks 2014-09-14 21:45 ` David Miller 2014-09-15 10:18 ` Alexander Y. Fomichev @ 2014-09-15 10:22 ` Alexander Fomichev 2014-09-15 18:25 ` David Miller 1 sibling, 1 reply; 7+ messages in thread From: Alexander Fomichev @ 2014-09-15 10:22 UTC (permalink / raw) To: netdev; +Cc: David Miller, Cong Wang, Vlad Yasevich, Alexander Fomichev __netdev_adjacent_dev_insert may add adjust device of different net namespace, without proper check it leads to emergence of broken sysfs links from/to devices in another namespace. Fix: rewrite netdev_adjacent_is_neigh_list macro as a function, move net_eq check into netdev_adjacent_is_neigh_list. (thanks David) related to: 4c75431ac3520631f1d9e74aa88407e6374dbbc4 Signed-off-by: Alexander Fomichev <git.user@gmail.com> --- net/core/dev.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index ab9a16530c36..00f15bc0a2ee 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4809,9 +4809,14 @@ static void netdev_adjacent_sysfs_del(struct net_device *dev, sysfs_remove_link(&(dev->dev.kobj), linkname); } -#define netdev_adjacent_is_neigh_list(dev, dev_list) \ - (dev_list == &dev->adj_list.upper || \ - dev_list == &dev->adj_list.lower) +static inline bool netdev_adjacent_is_neigh_list(struct net_device *dev, + struct net_device *adj_dev, + struct list_head *dev_list) +{ + return (dev_list == &dev->adj_list.upper || + dev_list == &dev->adj_list.lower) && + net_eq(dev_net(dev), dev_net(adj_dev)); +} static int __netdev_adjacent_dev_insert(struct net_device *dev, struct net_device *adj_dev, @@ -4841,7 +4846,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, pr_debug("dev_hold for %s, because of link added from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); - if (netdev_adjacent_is_neigh_list(dev, dev_list)) { + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) { ret = netdev_adjacent_sysfs_add(dev, adj_dev, dev_list); if (ret) goto free_adj; @@ -4862,7 +4867,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, return 0; remove_symlinks: - if (netdev_adjacent_is_neigh_list(dev, dev_list)) + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); free_adj: kfree(adj); @@ -4895,8 +4900,7 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev, if (adj->master) sysfs_remove_link(&(dev->dev.kobj), "master"); - if (netdev_adjacent_is_neigh_list(dev, dev_list) && - net_eq(dev_net(dev),dev_net(adj_dev))) + if (netdev_adjacent_is_neigh_list(dev, adj_dev, dev_list)) netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list); list_del_rcu(&adj->list); -- 2.1.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fix creation adjacent device symlinks 2014-09-15 10:22 ` [PATCH net] net: fix creation adjacent device symlinks Alexander Fomichev @ 2014-09-15 18:25 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2014-09-15 18:25 UTC (permalink / raw) To: git.user; +Cc: netdev, cwang, vyasevic From: Alexander Fomichev <git.user@gmail.com> Date: Mon, 15 Sep 2014 14:22:35 +0400 > __netdev_adjacent_dev_insert may add adjust device of different net > namespace, without proper check it leads to emergence of broken > sysfs links from/to devices in another namespace. > Fix: rewrite netdev_adjacent_is_neigh_list macro as a function, > move net_eq check into netdev_adjacent_is_neigh_list. > (thanks David) > related to: 4c75431ac3520631f1d9e74aa88407e6374dbbc4 > > Signed-off-by: Alexander Fomichev <git.user@gmail.com> Applied, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: fix sysfs symlinks of adjacent devices 2014-09-12 10:13 [PATCH net] net: fix sysfs symlinks of adjacent devices Alexander Fomichev 2014-09-12 13:33 ` Vlad Yasevich @ 2014-09-19 8:59 ` Andres Freund 1 sibling, 0 replies; 7+ messages in thread From: Andres Freund @ 2014-09-19 8:59 UTC (permalink / raw) To: Alexander Fomichev; +Cc: netdev, David Miller, Cong Wang, Vlad Yasevich Hi, On 2014-09-12 14:13:46 +0400, Alexander Fomichev wrote: > From: "Alexander Y. Fomichev" <git.user@gmail.com> > > __netdev_adjacent_dev_insert may add adjacent device from another > namespace. Without proper check it leads to emergence of broken > symlink from/to device not existing in current namespace. > Fix: check net_ns is the same before netdev_adjacent_sysfs_add/del > related to: 4c75431ac3520631f1d9e74aa88407e6374dbbc4 > This version, applied on top of 8ba4caf1ee, fixes the bug I had reported. Not just the testcase, but the actual usage scenario. I haven't tested David's version, but it doesn't look likely to be materially different. Greetings, Andres Freund ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-19 8:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-12 10:13 [PATCH net] net: fix sysfs symlinks of adjacent devices Alexander Fomichev 2014-09-12 13:33 ` Vlad Yasevich 2014-09-14 21:45 ` David Miller 2014-09-15 10:18 ` Alexander Y. Fomichev 2014-09-15 10:22 ` [PATCH net] net: fix creation adjacent device symlinks Alexander Fomichev 2014-09-15 18:25 ` David Miller 2014-09-19 8:59 ` [PATCH net] net: fix sysfs symlinks of adjacent devices Andres Freund
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.