Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
> Something like this then? Untested. Looks right to me, and seems to work. So I'll apply this. - R. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
Something like this then? Untested. Looks right to me, and seems to work. So I'll apply this. - R. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
> Quoting Adrian Bunk <[EMAIL PROTECTED]>:
> 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, >neigh_list, list) {
> kref_get(>ah->ref);
> neigh->ah = path->ah;
> memcpy(>dgid.raw, >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(>list);
> if (neigh->ah)
> ipoib_put_ah(neigh->ah);
> ipoib_neigh_free(dev, neigh);
> continue;
> }
> }
>
> while ((skb = __skb_dequeue(>queue)))
> __skb_queue_tail(, 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 <[EMAIL PROTECTED]>
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(>queue)))
__skb_queue_tail(, skb);
- list_for_each_entry(neigh, >neigh_list, list) {
+ list_for_each_entry_safe(neigh, t, >neigh_list, list) {
kref_get(>ah->ref);
neigh->ah = path->ah;
memcpy(>dgid.raw, >pathrec.dgid.raw,
--
MST
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
Quoting Adrian Bunk [EMAIL PROTECTED]:
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 [EMAIL PROTECTED]
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
