LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
@ 2007-03-19 9:23 Adrian Bunk
2007-03-19 9:46 ` [ofa-general] " Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Bunk @ 2007-03-19 9:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Roland Dreier, mshefty, halr, openib-general, linux-kernel
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.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
2007-03-19 9:23 drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free Adrian Bunk
@ 2007-03-19 9:46 ` Michael S. Tsirkin
2007-03-22 18:04 ` Roland Dreier
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2007-03-19 9:46 UTC (permalink / raw)
To: Adrian Bunk; +Cc: linux-kernel, openib-general
> Quoting Adrian Bunk <bunk@stusta.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@dev.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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [ofa-general] drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free
2007-03-19 9:46 ` [ofa-general] " Michael S. Tsirkin
@ 2007-03-22 18:04 ` Roland Dreier
0 siblings, 0 replies; 3+ messages in thread
From: Roland Dreier @ 2007-03-22 18:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Adrian Bunk, linux-kernel, openib-general
> Something like this then? Untested.
Looks right to me, and seems to work. So I'll apply this.
- R.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-03-22 18:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-19 9:23 drivers/infiniband/ulp/ipoib/ipoib_main.c: use-after-free Adrian Bunk
2007-03-19 9:46 ` [ofa-general] " Michael S. Tsirkin
2007-03-22 18:04 ` Roland Dreier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).