Message ID | 1410516826-6926-1-git-send-email-git.user@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);