* [PATCH v3 tip/core/rcu 12/14] bonding/bond_main: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
bond_change_active_slave(), bond_enslave(), and __bond_release_one()
are legitimate: They are assigning a pointer to an element from an
RCU-protected list (or a NULL pointer), and all elements of this list
are already visible to caller.
This commit therefore silences these false positives either by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett, or by using RCU_INIT_POINTER() for NULL pointer assignments.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
drivers/net/bonding/bond_main.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 72df399c4ab3..e4270ae1c0a8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -890,7 +890,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
if (new_active)
bond_set_slave_active_flags(new_active);
} else {
- rcu_assign_pointer(bond->curr_active_slave, new_active);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bond->curr_active_slave) = new_active;
}
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
@@ -1601,7 +1602,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
* so we can change it without calling change_active_interface()
*/
if (!bond->curr_active_slave && new_slave->link == BOND_LINK_UP)
- rcu_assign_pointer(bond->curr_active_slave, new_slave);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bond->curr_active_slave) = new_slave;
break;
} /* switch(bond_mode) */
@@ -1801,7 +1803,7 @@ static int __bond_release_one(struct net_device *bond_dev,
}
if (all) {
- rcu_assign_pointer(bond->curr_active_slave, NULL);
+ RCU_INIT_POINTER(bond->curr_active_slave, NULL);
} else if (oldcurrent == slave) {
/*
* Note that we hold RTNL over this sequence, so there
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 11/14] bridge/br_mdb: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
__br_mdb_del() is legitimate: They are assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences these false positives by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
net/bridge/br_mdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 85a09bb5ca51..de7197ba8695 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -447,7 +447,7 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
if (p->port->state == BR_STATE_DISABLED)
goto unlock;
- rcu_assign_pointer(*pp, p->next);
+ ACCESS_ONCE(*pp) = p->next; /* OK: Both --rcu and visible. */
hlist_del_init(&p->mglist);
del_timer(&p->timer);
call_rcu_bh(&p->rcu, br_multicast_free_pg);
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 10/14] mac80211: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, John W. Linville, Johannes Berg,
David S. Miller, linux-wireless, netdev
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
sta_info_hash_del() are legitimate: They are assigning a pointer to an
element from an RCU-protected list, and all elements of this list are
already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
net/mac80211/sta_info.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index aeb967a0aeed..494f03c0831f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -74,8 +74,8 @@ static int sta_info_hash_del(struct ieee80211_local *local,
if (!s)
return -ENOENT;
if (s == sta) {
- rcu_assign_pointer(local->sta_hash[STA_HASH(sta->sta.addr)],
- s->hnext);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(local->sta_hash[STA_HASH(sta->sta.addr)]) = s->hnext;
return 0;
}
@@ -84,7 +84,8 @@ static int sta_info_hash_del(struct ieee80211_local *local,
s = rcu_dereference_protected(s->hnext,
lockdep_is_held(&local->sta_mtx));
if (rcu_access_pointer(s->hnext)) {
- rcu_assign_pointer(s->hnext, sta->hnext);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(s->hnext) = sta->hnext;
return 0;
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 09/14] ipv6/sit: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ipip6_tunnel_unlink() is legitimate: It is assigning a pointer to an
element from an RCU-protected list, and all elements of this list are
already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv6/sit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 7ee5cb96db34..9b976a4b463d 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -157,7 +157,8 @@ static void ipip6_tunnel_unlink(struct sit_net *sitn, struct ip_tunnel *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 07/14] ipv6/ip6_tunnel: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ip6_tnl_unlink() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv6/ip6_tunnel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 61355f7f4da5..2bea7a4e49ed 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -245,7 +245,8 @@ ip6_tnl_unlink(struct ip6_tnl_net *ip6n, struct ip6_tnl *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 06/14] ipv4/ip_socketglue: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ip_ra_control() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv4/ip_sockglue.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index d9c4f113d709..a0e7f176e9c8 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -269,7 +269,8 @@ int ip_ra_control(struct sock *sk, unsigned char on,
}
/* dont let ip_call_ra_chain() use sk again */
ra->sk = NULL;
- rcu_assign_pointer(*rap, ra->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*rap) = ra->next;
spin_unlock_bh(&ip_ra_lock);
if (ra->destructor)
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 05/14] decnet: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Thomas Graf, Gao feng,
Stephen Hemminger, linux-decnet-user, netdev
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
dn_insert_route() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: linux-decnet-user@lists.sourceforge.net
Cc: netdev@vger.kernel.org
---
net/decnet/dn_route.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index fe32388ea24f..a6ef8b025035 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -344,8 +344,9 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
if (compare_keys(&rth->fld, &rt->fld)) {
/* Put it first */
*rthp = rth->dst.dn_next;
- rcu_assign_pointer(rth->dst.dn_next,
- dn_rt_hash_table[hash].chain);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(rth->dst.dn_next) =
+ dn_rt_hash_table[hash].chain;
rcu_assign_pointer(dn_rt_hash_table[hash].chain, rth);
dst_use(&rth->dst, now);
@@ -358,7 +359,8 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
rthp = &rth->dst.dn_next;
}
- rcu_assign_pointer(rt->dst.dn_next, dn_rt_hash_table[hash].chain);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(rt->dst.dn_next) = dn_rt_hash_table[hash].chain;
rcu_assign_pointer(dn_rt_hash_table[hash].chain, rt);
dst_use(&rt->dst, now);
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 04/14] wireless: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
cfg80211_combine_bsses() and cfg80211_bss_update() are legitimate:
They are assigning a pointer to an element from an RCU-protected list,
and all elements of this list are already visible to caller.
This commit therefore silences these false positives by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
net/wireless/scan.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index eeb71480f1af..ac3a47abf195 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -670,8 +670,8 @@ static bool cfg80211_combine_bsses(struct cfg80211_registered_device *dev,
list_add(&bss->hidden_list, &new->hidden_list);
bss->pub.hidden_beacon_bss = &new->pub;
new->refcount += bss->refcount;
- rcu_assign_pointer(bss->pub.beacon_ies,
- new->pub.beacon_ies);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bss->pub.beacon_ies) = new->pub.beacon_ies;
}
return true;
@@ -705,11 +705,12 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
old = rcu_access_pointer(found->pub.proberesp_ies);
- rcu_assign_pointer(found->pub.proberesp_ies,
- tmp->pub.proberesp_ies);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(found->pub.proberesp_ies) =
+ tmp->pub.proberesp_ies;
/* Override possible earlier Beacon frame IEs */
- rcu_assign_pointer(found->pub.ies,
- tmp->pub.proberesp_ies);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(found->pub.ies) = tmp->pub.proberesp_ies;
if (old)
kfree_rcu((struct cfg80211_bss_ies *)old,
rcu_head);
@@ -739,13 +740,14 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
old = rcu_access_pointer(found->pub.beacon_ies);
- rcu_assign_pointer(found->pub.beacon_ies,
- tmp->pub.beacon_ies);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(found->pub.beacon_ies) = tmp->pub.beacon_ies;
/* Override IEs if they were from a beacon before */
if (old == rcu_access_pointer(found->pub.ies))
- rcu_assign_pointer(found->pub.ies,
- tmp->pub.beacon_ies);
+ /* Both --rcu & visible, ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(found->pub.ies) =
+ tmp->pub.beacon_ies;
/* Assign beacon IEs to all sub entries */
list_for_each_entry(bss, &found->hidden_list,
@@ -755,8 +757,9 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
ies = rcu_access_pointer(bss->pub.beacon_ies);
WARN_ON(ies != old);
- rcu_assign_pointer(bss->pub.beacon_ies,
- tmp->pub.beacon_ies);
+ /* Both --rcu & visible, ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bss->pub.beacon_ies) =
+ tmp->pub.beacon_ies;
}
if (old)
@@ -803,8 +806,9 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
list_add(&new->hidden_list,
&hidden->hidden_list);
hidden->refcount++;
- rcu_assign_pointer(new->pub.beacon_ies,
- hidden->pub.beacon_ies);
+ /* Both --rcu & visible, ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(new->pub.beacon_ies) =
+ hidden->pub.beacon_ies;
}
} else {
/*
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 03/14] bridge: Apply ACCESS_ONCE() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-11 23:17 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
In-Reply-To: <1381533451-29018-1-git-send-email-paulmck@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
br_multicast_del_pg() and br_multicast_new_port_group() are legitimate:
They are assigning a pointer to an element from an RCU-protected list,
and all elements of this list are already visible to caller.
This commit therefore silences these false positives by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
net/bridge/br_multicast.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d1c578630678..bcc4bbc16498 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -267,7 +267,7 @@ static void br_multicast_del_pg(struct net_bridge *br,
if (p != pg)
continue;
- rcu_assign_pointer(*pp, p->next);
+ ACCESS_ONCE(*pp) = p->next; /* OK: Both --rcu and visible. */
hlist_del_init(&p->mglist);
del_timer(&p->timer);
call_rcu_bh(&p->rcu, br_multicast_free_pg);
@@ -646,7 +646,7 @@ struct net_bridge_port_group *br_multicast_new_port_group(
p->addr = *group;
p->port = port;
p->state = state;
- rcu_assign_pointer(p->next, next);
+ ACCESS_ONCE(p->next) = next; /* OK: Both --rcu and visible. */
hlist_add_head(&p->mglist, &port->mglist);
setup_timer(&p->timer, br_multicast_port_group_expired,
(unsigned long)p);
--
1.8.1.5
^ permalink raw reply related
* [PATCH v3 tip/core/rcu 0/14] Sparse-related updates for 3.13
From: Paul E. McKenney @ 2013-10-11 23:16 UTC (permalink / raw)
To: linux-kernel
Cc: peterz, fweisbec, dhowells, edumazet, gaofeng, mingo, bridge,
jmorris, dipankar, darren, josh, rostedt, niv, mathieu.desnoyers,
kuznet, tglx, johannes, laijs, yoshfuji, netdev,
linux-decnet-user, kaber, stephen, sbw, tgraf, akpm, fengguang.wu,
davem
Hello!
This series features updates to allow sparse to do a better job of
statically analyzing RCU usage:
1. Add a comment indicating that despite appearances,
rcu_assign_pointer() really only evaluates its arguments once,
as a cpp macro should.
2-13. Apply ACCESS_ONCE() to avoid a number of rcu_assign_pointer()
calls that would otherwise suffer sparse false positives given
patch #13 below.
14. Apply ACCESS_ONCE() to rcu_assign_pointer()'s target to prevent
comiler mischief. Also require that the source pointer be from
the kernel address space. Sometimes it can be from the RCU address
space, which necessitates the remaining patches in this series.
Which, it must be admitted, apply to a very small fraction of
the rcu_assign_pointer() invocations in the kernel. This commit
courtesy of Josh Triplett.
Changes from v2:
o Switch from rcu_assign_pointer() to ACCESS_ONCE() given that
the pointers are all --rcu and already visible to readers,
as suggested by Eric Dumazet and Josh Triplett.
o Place the commit adding the rcu_assign_pointer()'s ACCESS_ONCE()
at the end to allow better bisectability, as suggested by Josh
Triplett.
o Add a comment to rcu_assign_pointer() noting that it only evaluates
its arguments once, as suggested by Josh Triplett.
Changes from v1:
o Fix grammar nit in commit logs.
Thanx, Paul
b/drivers/net/bonding/bond_alb.c | 3 ++-
b/drivers/net/bonding/bond_main.c | 8 +++++---
b/include/linux/rcupdate.h | 20 +++++++++++++++++++-
b/kernel/notifier.c | 3 ++-
b/net/bridge/br_mdb.c | 2 +-
b/net/bridge/br_multicast.c | 4 ++--
b/net/decnet/dn_route.c | 8 +++++---
b/net/ipv4/ip_sockglue.c | 3 ++-
b/net/ipv6/ip6_gre.c | 3 ++-
b/net/ipv6/ip6_tunnel.c | 3 ++-
b/net/ipv6/sit.c | 3 ++-
b/net/mac80211/sta_info.c | 7 ++++---
b/net/wireless/scan.c | 32 ++++++++++++++++++--------------
13 files changed, 66 insertions(+), 33 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: fix vport-netdev unregister
From: Alexei Starovoitov @ 2013-10-11 22:48 UTC (permalink / raw)
To: Jesse Gross
Cc: Pravin Shelar, David S. Miller, Jiri Pirko, dev@openvswitch.org,
netdev
In-Reply-To: <CAEP_g=8JaTstBhpSSJnbFiLZ4V+jKwpomJy4ZSCe2trhYJBzTw@mail.gmail.com>
On Fri, Oct 11, 2013 at 3:02 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Fri, Oct 11, 2013 at 1:03 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On Fri, Oct 11, 2013 at 11:11 AM, Jesse Gross <jesse@nicira.com> wrote:
>>> On Thu, Oct 10, 2013 at 9:48 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>> On Thu, Oct 10, 2013 at 8:56 PM, Jesse Gross <jesse@nicira.com> wrote:
>>>>> However, the check dev->reg_state in netdev_destroy() looks racy to
>>>>> me, as it could already be in NETREG_UNREGISTERED even if we already
>>>>> processed this device.
>>>>
>>>> you mean that netdev_destroy() will see reg_state == netreg_unregistered,
>>>> while dp_device_event() didn't see reg_state == netreg_unregistering yet?
>>>> or dp_device_event() saw it, proceeded to do unlink and
>>>> netdev_destroy() ran in parallel?
>>>> well, that's why reg_state == netreg_unregistering check in netdev_destroy()
>>>> is done with rtnl_lock() held.
>>>> reg_state cannot go into netreg_unregistered state skipping
>>>> netreg_unregistering and notifier.
>>>> therefore I don't think it's racy.
>>>>
>>>> In ovs_dp_notify_wq() you're checking for both unregistering and
>>>> unregistered and that makes
>>>> sense, since workq can run after unregistering notifier called and
>>>> netdev_run_todo()
>>>> already changed the state to unregistered.
>>>> But here it's not the case.
>>>
>>> ovs_dp_notify_wq() calls ovs_dp_detach_port(), which indirectly calls
>>> netdev_destroy() so it seems like it actually is the same case to me.
>>
>> yes. makes sense.
>> how about:
>> - if (netdev_vport->dev->reg_state != NETREG_UNREGISTERING)
>> + if (netdev_vport->dev->priv_flags & IFF_OVS_DATAPATH)
>
> Yes, this seems safer. Is the check for NETREG_UNREGISTERING in
> dp_device_event() still needed given that we are checking the event?
at least some check is needed, since NETDEV_UNREGISTER event can
be received again as rebroadcast with reg_state=netreg_unregistered
if wq got delayed.
Probably better to combine checks event == unreg and state == unregistering
under one 'if' to avoid unnecessary workq wakeup.
Or may be better to do it as
if (event == NETDEV_UNREGISTER && dev->priv_flags & IFF_OVS_DATAPATH) {
ovs_netdev_detach_dev();
queue_work();
}
since we're at it... what should be the behavior for namespace moves?
If dev attached to ovs and being moved into a different net namespace, I think
ovs should detach and forget the dev...
Today ovs ignores this notification and we may have ovs-dp in one net
and attached dev
in a different net.
So if you do:
ovs-dpctl add-if test tap1
ip link set tap1 netns 3512
and then try to remove tap1 inside the namespace:
ip tuntap del dev tap1 mode tap
it will just hang:
[ 852.572476] unregister_netdevice: waiting for tap1 to become free.
Usage count = 3
[ 862.578769] unregister_netdevice: waiting for tap1 to become free.
Usage count = 3
>> ovs_netdev_destroy_dev() name instead ovs_netdev_unlink_dev() name?
>
> How about detach_dev?
that's better name indeed. Will respin V2.
Thanks
Alexei
^ permalink raw reply
* Re: [PATCH net-next] tcp: tcp_transmit_skb() optimizations
From: Eric Dumazet @ 2013-10-11 22:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fitz, ycheng, ncardwell
In-Reply-To: <20131011.174858.1461090000704613432.davem@davemloft.net>
On Fri, 2013-10-11 at 17:48 -0400, David Miller wrote:
> This patch is correct, so I've applied it, but it points out a bug.
>
> The __tcp_retransmit_skb() code that does a __pskb_copy() to handle
> NET_IP_ALIGN violations and skb_headroom() overflows is buggy because
> it needs to store a congestion control timestamp in the original 'skb'
> since that's what we'll look at in the retransmit queue.
Yes, I saw that, indeed.
I added it as low priority bug for the moment, as the default congestion
module do not really care, and this case is really unlikely ;)
^ permalink raw reply
* 3.10 stable request: iwlwifi: add new 7260 and 3160 series device IDs
From: Bjørn Mork @ 2013-10-11 22:26 UTC (permalink / raw)
To: netdev; +Cc: Oren Givon, Johannes Berg
Hello,
could you please add this commit to your 3.10 stable queue?
commit 93fc64114b994f9ef6901697f9b0de00762680e9
Author: Oren Givon <oren.givon@intel.com>
Date: Tue Apr 23 18:19:11 2013 +0300
iwlwifi: add new 7260 and 3160 series device IDs
Add new device IDs and configurations to support
all the devices.
Signed-off-by: Oren Givon <oren.givon@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
I just installed Debian testing on a Sony Vaio Pro and that was a bit
more hassle than necessary due to this patch missing in their 3.10.11
based kernel...
The patch applies cleanly on top of v3.10.15, and only add new device
IDs, so it should be a clear stable candidate.
Thanks,
Bjørn
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: fix vport-netdev unregister
From: Jesse Gross @ 2013-10-11 22:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, Jiri Pirko,
David S. Miller
In-Reply-To: <CAMEtUuwAJr0uMv4b_SfYG378p06os3bvT3kyvVGpKguAZbNcSw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Oct 11, 2013 at 1:03 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Oct 11, 2013 at 11:11 AM, Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org> wrote:
>> On Thu, Oct 10, 2013 at 9:48 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>> On Thu, Oct 10, 2013 at 8:56 PM, Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org> wrote:
>>>> However, the check dev->reg_state in netdev_destroy() looks racy to
>>>> me, as it could already be in NETREG_UNREGISTERED even if we already
>>>> processed this device.
>>>
>>> you mean that netdev_destroy() will see reg_state == netreg_unregistered,
>>> while dp_device_event() didn't see reg_state == netreg_unregistering yet?
>>> or dp_device_event() saw it, proceeded to do unlink and
>>> netdev_destroy() ran in parallel?
>>> well, that's why reg_state == netreg_unregistering check in netdev_destroy()
>>> is done with rtnl_lock() held.
>>> reg_state cannot go into netreg_unregistered state skipping
>>> netreg_unregistering and notifier.
>>> therefore I don't think it's racy.
>>>
>>> In ovs_dp_notify_wq() you're checking for both unregistering and
>>> unregistered and that makes
>>> sense, since workq can run after unregistering notifier called and
>>> netdev_run_todo()
>>> already changed the state to unregistered.
>>> But here it's not the case.
>>
>> ovs_dp_notify_wq() calls ovs_dp_detach_port(), which indirectly calls
>> netdev_destroy() so it seems like it actually is the same case to me.
>
> yes. makes sense.
> how about:
> - if (netdev_vport->dev->reg_state != NETREG_UNREGISTERING)
> + if (netdev_vport->dev->priv_flags & IFF_OVS_DATAPATH)
Yes, this seems safer. Is the check for NETREG_UNREGISTERING in
dp_device_event() still needed given that we are checking the event?
> ovs_netdev_destroy_dev() name instead ovs_netdev_unlink_dev() name?
How about detach_dev?
^ permalink raw reply
* Re: [PATCH 14/14] net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data
From: David Miller @ 2013-10-11 21:56 UTC (permalink / raw)
To: matthew.leach
Cc: linux-arm-kernel, ankit.jindal, steve.mcintyre, tushar.jagad,
will.deacon, catalin.marinas, netdev, nico
In-Reply-To: <1381499540-28794-15-git-send-email-matthew.leach@arm.com>
From: Matthew Leach <matthew.leach@arm.com>
Date: Fri, 11 Oct 2013 14:52:20 +0100
> From: Will Deacon <will.deacon@arm.com>
>
> SMC_outw invokes an endian-aware I/O accessor, which may change the data
> endianness before writing to the device. This is not suitable for data
> transfers where the memory buffer is simply a string of bytes that does
> not require any byte-swapping.
>
> This patches fixes the smc91x SMC_PUSH_DATA macro so that it uses the
> string I/O accessor for outputting the leading or trailing halfwords on
> halfword-aligned buffers.
>
> Cc: <netdev@vger.kernel.org>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
Applied.
^ permalink raw reply
* Re: [patch] farsync: fix info leak in ioctl
From: David Miller @ 2013-10-11 21:55 UTC (permalink / raw)
To: dan.carpenter; +Cc: kevin.curtis, speiro, security, netdev
In-Reply-To: <20131011095003.GD6247@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Fri, 11 Oct 2013 12:50:03 +0300
> From: Salva Peiró <speiro@ai2.upv.es>
>
> The fst_get_iface() code fails to initialize the two padding bytes of
> struct sync_serial_settings after the ->loopback member. Add an explicit
> memset(0) before filling the structure to avoid the info leak.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] ipv6: Initialize ip6_tnl.hlen in gre tunnel even if no route is found
From: David Miller @ 2013-10-11 21:55 UTC (permalink / raw)
To: hannes; +Cc: ou.ghorbel, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <20131011150216.GA18601@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 11 Oct 2013 17:02:17 +0200
> On Thu, Oct 10, 2013 at 06:50:27PM +0100, Oussama Ghorbel wrote:
>> The ip6_tnl.hlen (gre and ipv6 headers length) is independent from the
>> outgoing interface, so it would be better to initialize it even when no
>> route is found, otherwise its value will be zero.
>> While I'm not sure if this could happen in real life, but doing that
>> will avoid to call the skb_push function with a zero in ip6gre_header
>> function.
>>
>> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Signed-off-by: Oussama Ghorbel <ou.ghorbel@gmail.com>
>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] tcp: tcp_transmit_skb() optimizations
From: David Miller @ 2013-10-11 21:48 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, fitz, ycheng, ncardwell
In-Reply-To: <1381419780.4971.84.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 Oct 2013 08:43:00 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> 1) We need to take a timestamp only for skb that should be cloned.
>
> Other skbs are not in write queue and no rtt estimation is done on them.
>
> 2) the unlikely() hint is wrong for receivers (they send pure ACK)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
This patch is correct, so I've applied it, but it points out a bug.
The __tcp_retransmit_skb() code that does a __pskb_copy() to handle
NET_IP_ALIGN violations and skb_headroom() overflows is buggy because
it needs to store a congestion control timestamp in the original 'skb'
since that's what we'll look at in the retransmit queue.
^ permalink raw reply
* Re: [PATCH net] netem: free skb's in tree on reset
From: David Miller @ 2013-10-11 21:31 UTC (permalink / raw)
To: stephen; +Cc: eric.dumazet, netdev
In-Reply-To: <20131006151649.38038c0e@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 6 Oct 2013 15:16:49 -0700
> Netem can leak memory because packets get stored in red-black
> tree and it is not cleared on reset.
>
> Reported by: Сергеев Сергей <adron@yapic.net>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] netem: update backlog after drop
From: David Miller @ 2013-10-11 21:31 UTC (permalink / raw)
To: stephen; +Cc: eric.dumazet, netdev
In-Reply-To: <20131006151533.52988624@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 6 Oct 2013 15:15:33 -0700
> When packet is dropped from rb-tree netem the backlog statistic should
> also be updated.
>
> Reported-by: Сергеев Сергей <adron@yapic.net>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied.
^ permalink raw reply
* Re: [PATCH] l2tp: must disable bh before calling l2tp_xmit_skb()
From: David Miller @ 2013-10-11 21:20 UTC (permalink / raw)
To: eric.dumazet; +Cc: f.cachereul, jchapman, netdev
In-Reply-To: <1381411809.4971.77.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 10 Oct 2013 06:30:09 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> François Cachereul made a very nice bug report and suspected
> the bh_lock_sock() / bh_unlok_sock() pair used in l2tp_xmit_skb() from
> process context was not good.
>
> This problem was added by commit
> ("l2tp: Fix locking in l2tp_core.c").
I added the commit SHA ID for you, please take care of this next time.
> l2tp_eth_dev_xmit() runs from BH context, so we must disable BH
> from other l2tp_xmit_skb() users.
...
> Reported-by: François Cachereul <f.cachereul@alphalink.fr>
> Tested-by: François Cachereul <f.cachereul@alphalink.fr>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* RE: [PATCH 1/1] net: fix cipso packet validation when !NETLABEL
From: Seif Mazareeb @ 2013-10-11 21:04 UTC (permalink / raw)
To: Paul Moore
Cc: davem@davemloft.net, netdev@vger.kernel.org,
thomas.petazzoni@free-electrons.com, Dmitri Epshtein
In-Reply-To: <11516872.z0JUlZSHlI@sifl>
When CONFIG_NETLABEL is disabled, the cipso_v4_validate() function could loop
forever in the main loop if opt[opt_iter +1] == 0, this will causing a kernel
crash in an SMP system, since the CPU executing this function will
stall /not respond to IPIs.
This problem can be reproduced by running the IP Stack Integrity Checker
(http://isic.sourceforge.net) using the following command on a Linux machine
connected to DUT:
"icmpsic -s rand -d <DUT IP address> -r 123456"
wait (1-2 min)
Signed-off-by: Seif Mazareeb <seif@marvell.com>
---
include/net/cipso_ipv4.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/net/cipso_ipv4.h b/include/net/cipso_ipv4.h
index a7a683e..286b7da 100644
--- a/include/net/cipso_ipv4.h
+++ b/include/net/cipso_ipv4.h
@@ -290,6 +290,7 @@ static inline int cipso_v4_validate(const struct sk_buff *skb,
unsigned char err_offset = 0;
u8 opt_len = opt[1];
u8 opt_iter;
+ u8 tag_len;
if (opt_len < 8) {
err_offset = 1;
@@ -302,7 +303,8 @@ static inline int cipso_v4_validate(const struct sk_buff *skb,
}
for (opt_iter = 6; opt_iter < opt_len;) {
- if (opt[opt_iter + 1] > (opt_len - opt_iter)) {
+ tag_len = opt[opt_iter + 1];
+ if ((tag_len == 0) || (opt[opt_iter + 1] > (opt_len - opt_iter))) {
err_offset = opt_iter + 1;
goto out;
}
--
1.8.1.2
-----Original Message-----
From: Paul Moore [mailto:paul@paul-moore.com]
Sent: Friday, October 11, 2013 12:02 PM
To: Seif Mazareeb
Cc: davem@davemloft.net; netdev@vger.kernel.org; thomas.petazzoni@free-electrons.com; Dmitri Epshtein
Subject: Re: [PATCH 1/1] net: fix cipso packet validation when !NETLABEL
On Friday, October 11, 2013 10:58:31 AM Seif Mazareeb wrote:
> When CONFIG_NETLABEL is disabled, the cipso_v4_validate() function
> could loop forever in the main loop if opt[opt_iter +1] == 0, this
> will causing a kernel crash in an SMP system, since the CPU executing
> this function will stall /not respond to IPIs.
>
> This problem can be reproduced by running the IP Stack Integrity
> Checker
> (http://isic.sourceforge.net) using the following command on a Linux
> machine connected to DUT:
>
> "icmpsic -s rand -d <DUT IP address> -r 123456"
> wait (1-2 min)
>
> Signed-off-by: Seif Mazareeb <seif@marvell.com>
> ---
> include/net/cipso_ipv4.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/net/cipso_ipv4.h b/include/net/cipso_ipv4.h index
> a7a683e..047f1f6 100644
> --- a/include/net/cipso_ipv4.h
> +++ b/include/net/cipso_ipv4.h
> @@ -306,6 +306,10 @@ static inline int cipso_v4_validate(const struct
> sk_buff *skb, err_offset = opt_iter + 1;
> goto out;
> }
> +
> + if (opt[opt_iter + 1] == 0)
> + break;
> +
> opt_iter += opt[opt_iter + 1];
> }
Thanks for finding and reporting this bug. Unfortunately, I don't think the supplied patch is the best way to solve this. Since a length of zero is not valid for any known CIPSO tag types (at least that I am aware of), we should treat a zero length tag as an error, similar to how we treat tags with length values that stretch beyond the option itself.
I'm thinking something like this:
static inline int cipso_v4_validate(const struct sk_buff *skb,
unsigned char **option) {
unsigned char *opt = *option;
unsigned char err_offset = 0;
u8 opt_len = opt[1];
u8 opt_iter;
u8 tag_len;
if (opt_len < 8) {
err_offset = 1;
goto out;
}
if (get_unaligned_be32(&opt[2]) == 0) {
err_offset = 2;
goto out;
}
for (opt_iter = 6; opt_iter < opt_len;) {
tag_len = opt[opt_iter + 1];
if ((tag_len == 0) || (tag_len > (opt_len - opt_iter))) {
err_offset = opt_iter + 1;
goto out;
}
opt_iter += tag_len;
}
out:
*option = opt + err_offset;
return err_offset;
}
If you want to fixup your patch that would be appreciated, if not, please let me know so I can submit the fix.
Thanks,
-Paul
--
paul moore
www.paul-moore.com
^ permalink raw reply related
* Re: pull request: batman-adv 2013-10-09b
From: David Miller @ 2013-10-11 20:45 UTC (permalink / raw)
To: antonio-x4xJYDvStAgysxA8WJXlww
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20131011061053.GF576-rVWd3aGhH2zPj3vggD0kEA@public.gmane.org>
From: Antonio Quartulli <antonio-x4xJYDvStAgysxA8WJXlww@public.gmane.org>
Date: Fri, 11 Oct 2013 08:10:53 +0200
> On Wed, Oct 09, 2013 at 03:56:58PM -0400, David Miller wrote:
>> From: Antonio Quartulli <antonio-x4xJYDvStAgysxA8WJXlww@public.gmane.org>
>> Date: Wed, 9 Oct 2013 21:32:38 +0200
>>
>> > here you have my fixed pull request intended for net-next.
>> >
>> > The previous build error was due to an accidental remotion of the beginning of a
>> > batadv_dbg() statement during a merge conflict resolution.
>> > Sorry for that.
>>
>> This looks better, pulled, thanks a lot.
>
> Hello David,
>
> I can't find my patchset in net-next, hasn't it been pushed yet?
Sorry, it should be there now.
^ permalink raw reply
* Re: [PATCH 14/14] net: smc91x: dont't use SMC_outw for fixing up halfword-aligned data
From: Nicolas Pitre @ 2013-10-11 20:41 UTC (permalink / raw)
To: Matthew Leach
Cc: linux-arm-kernel, ankit.jindal, steve.mcintyre, tushar.jagad,
will.deacon, catalin.marinas, netdev, David S. Miller
In-Reply-To: <1381499540-28794-15-git-send-email-matthew.leach@arm.com>
On Fri, 11 Oct 2013, Matthew Leach wrote:
> From: Will Deacon <will.deacon@arm.com>
>
> SMC_outw invokes an endian-aware I/O accessor, which may change the data
> endianness before writing to the device. This is not suitable for data
> transfers where the memory buffer is simply a string of bytes that does
> not require any byte-swapping.
>
> This patches fixes the smc91x SMC_PUSH_DATA macro so that it uses the
> string I/O accessor for outputting the leading or trailing halfwords on
> halfword-aligned buffers.
Acked-by: Nicolas Pitre <nico@linaro.org>
>
> Cc: <netdev@vger.kernel.org>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> drivers/net/ethernet/smsc/smc91x.h | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
> index 5730fe2..98eedb9 100644
> --- a/drivers/net/ethernet/smsc/smc91x.h
> +++ b/drivers/net/ethernet/smsc/smc91x.h
> @@ -1124,8 +1124,7 @@ static const char * chip_ids[ 16 ] = {
> void __iomem *__ioaddr = ioaddr; \
> if (__len >= 2 && (unsigned long)__ptr & 2) { \
> __len -= 2; \
> - SMC_outw(*(u16 *)__ptr, ioaddr, \
> - DATA_REG(lp)); \
> + SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
> __ptr += 2; \
> } \
> if (SMC_CAN_USE_DATACS && lp->datacs) \
> @@ -1133,8 +1132,7 @@ static const char * chip_ids[ 16 ] = {
> SMC_outsl(__ioaddr, DATA_REG(lp), __ptr, __len>>2); \
> if (__len & 2) { \
> __ptr += (__len & ~3); \
> - SMC_outw(*((u16 *)__ptr), ioaddr, \
> - DATA_REG(lp)); \
> + SMC_outsw(ioaddr, DATA_REG(lp), __ptr, 1); \
> } \
> } else if (SMC_16BIT(lp)) \
> SMC_outsw(ioaddr, DATA_REG(lp), p, (l) >> 1); \
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Mark Lord @ 2013-10-11 20:29 UTC (permalink / raw)
To: Alexander Gordeev
Cc: H. Peter Anvin, Benjamin Herrenschmidt, linux-kernel,
Bjorn Helgaas, Ralf Baechle, Michael Ellerman, Martin Schwidefsky,
Ingo Molnar, Tejun Heo, Dan Williams, Andy King, Jon Mason,
Matt Porter, linux-pci, linux-mips, linuxppc-dev, linux390,
linux-s390, x86, linux-ide, iss_storagedev, linux-nvme,
linux-rdma, netdev, e1000-dev
In-Reply-To: <20131011084108.GA25702@dhcp-26-207.brq.redhat.com>
On 13-10-11 04:41 AM, Alexander Gordeev wrote:
> On Thu, Oct 10, 2013 at 07:17:18PM -0400, Mark Lord wrote:
>> Just to help us all understand "the loop" issue..
>>
>> Here's an example of driver code which uses the existing MSI-X interfaces,
>> for a device which can work with either 16, 8, 4, 2, or 1 MSI-X interrupt.
>> This is from a new driver I'm working on right now:
..
> Now, this is a loop-free alternative:
>
> static int xx_alloc_msix_irqs(struct xx_dev *dev, int nvec)
> {
> nvec = roundup_pow_of_two(nvec); /* assume 0 > nvec <= 16 */
>
> xx_disable_all_irqs(dev);
>
> pci_lock_msi(dev->pdev);
>
> rc = pci_get_msix_limit(dev->pdev, nvec);
> if (rc < 0)
> goto err;
>
> nvec = min(nvec, rc); /* if limit is more than requested */
> nvec = rounddown_pow_of_two(nvec); /* (a) */
>
> xx_prep_for_msix_vectors(dev, nvec);
>
> rc = pci_enable_msix(dev->pdev, dev->irqs, nvec); /* (b) */
> if (rc < 0)
> goto err;
>
> pci_unlock_msi(dev->pdev);
>
> dev->num_vectors = nvec; /* (b) */
> return 0;
>
> err:
> pci_unlock_msi(dev->pdev);
>
> kerr(dev->name, "pci_enable_msix() failed, err=%d", rc);
> dev->num_vectors = 0;
> return rc;
> }
That would still need a loop, to handle the natural race between
the calls to pci_get_msix_limit() and pci_enable_msix() -- the driver and device
can and should fall back to a smaller number of vectors when pci_enable_msix() fails.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox