linux-kernel - Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070319094619.GE8386@mellanox.co.il>
Date:	Mon, 19 Mar 2007 11:46:19 +0200
From:	"Michael S. Tsirkin" <mst@....mellanox.co.il>
To:	Adrian Bunk <bunk@...sta.de>
Cc:	linux-kernel@...r.kernel.org, openib-general@...nib.org
Subject: Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c:
	use-after-free

> Quoting Adrian Bunk <bunk@...sta.de>:
> Subject: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
> 
> The Coverity checker spotted the following code introduced by
> commit 839fcaba355abaffb7b44f0f4504093acb0b11cf:
> 
> <--  snip  -->
> 
> ...
> static void path_rec_completion(int status,
>                                 struct ib_sa_path_rec *pathrec,
>                                 void *path_ptr)
> {
> ...
>                 list_for_each_entry(neigh, &path->neigh_list, list) {
>                         kref_get(&path->ah->ref);
>                         neigh->ah = path->ah;
>                         memcpy(&neigh->dgid.raw, &path->pathrec.dgid.raw,
>                                sizeof(union ib_gid));
> 
>                         if (ipoib_cm_enabled(dev, neigh->neighbour)) {
>                                 if (!ipoib_cm_get(neigh))
>                                         ipoib_cm_set(neigh, ipoib_cm_create_tx(dev,
>                                                                                path,
>                                                                                neigh));
>                                 if (!ipoib_cm_get(neigh)) {
>                                         list_del(&neigh->list);
>                                         if (neigh->ah)
>                                                 ipoib_put_ah(neigh->ah);
>                                         ipoib_neigh_free(dev, neigh);
>                                         continue;
>                                 }
>                         }
> 
>                         while ((skb = __skb_dequeue(&neigh->queue)))
>                                 __skb_queue_tail(&skqueue, skb);
>                 }
> ...
> 
> <--  snip  -->
> 
> Notice that before the continue "neigh" gets freed, but the 
> list_for_each_entry() for() loop uses it.

Something like this then? Untested.

Signed-off-by: Michael S. Tsirkin <mst@....mellanox.co.il>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 12b528b..706eb88 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -380,7 +380,7 @@ static void path_rec_completion(int status,
 	struct net_device *dev = path->dev;
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
 	struct ipoib_ah *ah = NULL;
-	struct ipoib_neigh *neigh;
+	struct ipoib_neigh *neigh, *t;
 	struct sk_buff_head skqueue;
 	struct sk_buff *skb;
 	unsigned long flags;
@@ -418,7 +418,7 @@ static void path_rec_completion(int status,
 		while ((skb = __skb_dequeue(&path->queue)))
 			__skb_queue_tail(&skqueue, skb);
 
-		list_for_each_entry(neigh, &path->neigh_list, list) {
+		list_for_each_entry_safe(neigh, t, &path->neigh_list, list) {
 			kref_get(&path->ah->ref);
 			neigh->ah = path->ah;
 			memcpy(&neigh->dgid.raw, &path->pathrec.dgid.raw,

-- 
MST
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/