* [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 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).