| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1322602283.2596.25.camel@edumazet-laptop>
Date: Tue, 29 Nov 2011 22:31:23 +0100
From: Eric Dumazet <eric.dumazet@...il.com>
To: Marc Aurele La France <tsi@...berta.ca>
Cc: Roland Dreier <roland@...nel.org>,
David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
linux-rdma@...r.kernel.org
Subject: Re: RCU'ed dst_get_neighbour()
Le mardi 29 novembre 2011 à 22:17 +0100, Eric Dumazet a écrit :
> Le mardi 29 novembre 2011 à 14:00 -0700, Marc Aurele La France a écrit :
> > On Tue, 29 Nov 2011, Eric Dumazet wrote:
> >
> > > Le mardi 29 novembre 2011 à 12:47 -0800, Roland Dreier a écrit :
> > >> Thanks Eric, I'll send this to Linus shortly.
> >
> > > Oh well, I forgot one rcu_read_unlock(), I'll send a V2...
> >
> > This also doesn't address the other dst_get_neighbour() instances
> > introduced by
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=69cce1d1404968f78b177a0314f5822d5afdbbfb
> >
>
> Oh well, a complete audit is needed, and I have no choice but doing it.
>
> Thanks !
>
Here is the result of this audit, please double check and test it, I
only compiled this.
Thanks !
[PATCH V2] drivers/infiniband: fix lockdep splats
commit f2c31e32b37 (net: fix NULL dereferences in check_peer_redir())
forgot to take care of infiniband uses of dst neighbours.
Many thanks to Marc Aurele who provided a nice bug report and feedback.
Reported-by: Marc Aurele La France <tsi@...berta.ca>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
CC: David Miller <davem@...emloft.net>
CC: Roland Dreier <roland@...nel.org>
---
drivers/infiniband/core/addr.c | 9 +++++--
drivers/infiniband/hw/cxgb3/iwch_cm.c | 4 +++
drivers/infiniband/hw/cxgb4/cm.c | 6 +++++
drivers/infiniband/hw/nes/nes_cm.c | 6 +++--
drivers/infiniband/ulp/ipoib/ipoib_main.c | 18 +++++++++------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 +++--
6 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 691276b..e9cf51b 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -216,7 +216,9 @@ static int addr4_resolve(struct sockaddr_in *src_in,
neigh = neigh_lookup(&arp_tbl, &rt->rt_gateway, rt->dst.dev);
if (!neigh || !(neigh->nud_state & NUD_VALID)) {
+ rcu_read_lock();
neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
+ rcu_read_unlock();
ret = -ENODATA;
if (neigh)
goto release;
@@ -274,15 +276,16 @@ static int addr6_resolve(struct sockaddr_in6 *src_in,
goto put;
}
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
if (!neigh || !(neigh->nud_state & NUD_VALID)) {
if (neigh)
neigh_event_send(neigh, NULL);
ret = -ENODATA;
- goto put;
+ } else {
+ ret = rdma_copy_addr(addr, dst->dev, neigh->ha);
}
-
- ret = rdma_copy_addr(addr, dst->dev, neigh->ha);
+ rcu_read_unlock();
put:
dst_release(dst);
return ret;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index de6d077..c88b12b 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -1375,8 +1375,10 @@ static int pass_accept_req(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
goto reject;
}
dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
l2t = t3_l2t_get(tdev, neigh, neigh->dev);
+ rcu_read_unlock();
if (!l2t) {
printk(KERN_ERR MOD "%s - failed to allocate l2t entry!\n",
__func__);
@@ -1946,10 +1948,12 @@ int iwch_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
ep->l2t = t3_l2t_get(ep->com.tdev, neigh, neigh->dev);
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index b36cdac..75b57be 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -1594,6 +1594,7 @@ static int pass_accept_req(struct c4iw_dev *dev, struct sk_buff *skb)
goto reject;
}
dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(dst);
if (neigh->dev->flags & IFF_LOOPBACK) {
pdev = ip_dev_find(&init_net, peer_ip);
@@ -1620,6 +1621,7 @@ static int pass_accept_req(struct c4iw_dev *dev, struct sk_buff *skb)
rss_qid = dev->rdev.lldi.rxq_ids[
cxgb4_port_idx(neigh->dev) * step];
}
+ rcu_read_unlock();
if (!l2t) {
printk(KERN_ERR MOD "%s - failed to allocate l2t entry!\n",
__func__);
@@ -1820,6 +1822,7 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
@@ -1856,6 +1859,7 @@ static int c4iw_reconnect(struct c4iw_ep *ep)
ep->rss_qid = ep->com.dev->rdev.lldi.rxq_ids[
cxgb4_port_idx(neigh->dev) * step];
}
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
@@ -2301,6 +2305,7 @@ int c4iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
}
ep->dst = &rt->dst;
+ rcu_read_lock();
neigh = dst_get_neighbour(ep->dst);
/* get a l2t entry */
@@ -2339,6 +2344,7 @@ int c4iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
ep->retry_with_mpa_v1 = 0;
ep->tried_with_mpa_v1 = 0;
}
+ rcu_read_unlock();
if (!ep->l2t) {
printk(KERN_ERR MOD "%s - cannot alloc l2e.\n", __func__);
err = -ENOMEM;
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index dfce9ea..0a52d72 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -1377,9 +1377,11 @@ static int nes_addr_resolve_neigh(struct nes_vnic *nesvnic, u32 dst_ip, int arpi
neigh_release(neigh);
}
- if ((neigh == NULL) || (!(neigh->nud_state & NUD_VALID)))
+ if ((neigh == NULL) || (!(neigh->nud_state & NUD_VALID))) {
+ rcu_read_lock();
neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
-
+ rcu_read_unlock();
+ }
ip_rt_put(rt);
return rc;
}
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 7567b60..ef38848 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -555,6 +555,7 @@ static int path_rec_start(struct net_device *dev,
return 0;
}
+/* called with rcu_read_lock */
static void neigh_add_path(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -636,6 +637,7 @@ err_drop:
spin_unlock_irqrestore(&priv->lock, flags);
}
+/* called with rcu_read_lock */
static void ipoib_path_lookup(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(skb->dev);
@@ -720,13 +722,14 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct neighbour *n = NULL;
unsigned long flags;
+ rcu_read_lock();
if (likely(skb_dst(skb)))
n = dst_get_neighbour(skb_dst(skb));
if (likely(n)) {
if (unlikely(!*to_ipoib_neigh(n))) {
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
neigh = *to_ipoib_neigh(n);
@@ -749,17 +752,17 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
ipoib_neigh_free(dev, neigh);
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_path_lookup(skb, dev);
- return NETDEV_TX_OK;
+ goto unlock;
}
if (ipoib_cm_get(neigh)) {
if (ipoib_cm_up(neigh)) {
ipoib_cm_send(dev, skb, ipoib_cm_get(neigh));
- return NETDEV_TX_OK;
+ goto unlock;
}
} else if (neigh->ah) {
ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(n->ha));
- return NETDEV_TX_OK;
+ goto unlock;
}
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
@@ -793,13 +796,14 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
phdr->hwaddr + 4);
dev_kfree_skb_any(skb);
++dev->stats.tx_dropped;
- return NETDEV_TX_OK;
+ goto unlock;
}
unicast_arp_send(skb, dev, phdr);
}
}
-
+unlock:
+ rcu_read_unlock();
return NETDEV_TX_OK;
}
@@ -837,7 +841,7 @@ static int ipoib_hard_header(struct sk_buff *skb,
dst = skb_dst(skb);
n = NULL;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if ((!dst || !n) && daddr) {
struct ipoib_pseudoheader *phdr =
(struct ipoib_pseudoheader *) skb_push(skb, sizeof *phdr);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 1b7a976..cad1894 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -266,7 +266,7 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
skb->dev = dev;
if (dst)
- n = dst_get_neighbour(dst);
+ n = dst_get_neighbour_raw(dst);
if (!dst || !n) {
/* put pseudoheader back on for next time */
skb_push(skb, sizeof (struct ipoib_pseudoheader));
@@ -722,6 +722,8 @@ out:
if (mcast && mcast->ah) {
struct dst_entry *dst = skb_dst(skb);
struct neighbour *n = NULL;
+
+ rcu_read_lock();
if (dst)
n = dst_get_neighbour(dst);
if (n && !*to_ipoib_neigh(n)) {
@@ -734,7 +736,7 @@ out:
list_add_tail(&neigh->list, &mcast->neigh_list);
}
}
-
+ rcu_read_unlock();
spin_unlock_irqrestore(&priv->lock, flags);
ipoib_send(dev, skb, mcast->ah, IB_MULTICAST_QPN);
return;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists