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