IB/ipoib: Avoid memory leak if the SA returns a different DGID
authorErez Shitrit <erezsh@mellanox.com>
Tue, 14 Nov 2017 12:51:53 +0000 (14:51 +0200)
committerJason Gunthorpe <jgg@mellanox.com>
Mon, 18 Dec 2017 22:37:05 +0000 (15:37 -0700)
The ipoib path database is organized around DGIDs from the LLADDR, but the
SA is free to return a different GID when asked for path. This causes a
bug because the SA's modified DGID is copied into the database key, even
though it is no longer the correct lookup key, causing a memory leak and
other malfunctions.

Ensure the database key does not change after the SA query completes.

Demonstration of the bug is as  follows
ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
creates new record in the DB with that gid as a key, and issues a new
request to the SM.
Now, the SM from some reason returns path-record with other SGID (for
example, 2001:0000:0000:0000:0002:c903:00ef:5ee2 that contains the local
subnet prefix) now ipoib will overwrite the current entry with the new
one, and if new request to the original GID arrives ipoib  will not find
it in the DB (was overwritten) and will create new record that in its
turn will also be overwritten by the response from the SM, and so on
till the driver eats all the device memory.

Signed-off-by: Erez Shitrit <erezsh@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
drivers/infiniband/ulp/ipoib/ipoib_main.c

index 567dbd45817c60c10c02829c6912d19f66ecbbc0..7301a79c4058778688fb6ab4b104d3877f0fa469 100644 (file)
@@ -775,6 +775,22 @@ static void path_rec_completion(int status,
        spin_lock_irqsave(&priv->lock, flags);
 
        if (!IS_ERR_OR_NULL(ah)) {
+               /*
+                * pathrec.dgid is used as the database key from the LLADDR,
+                * it must remain unchanged even if the SA returns a different
+                * GID to use in the AH.
+                */
+               if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
+                          sizeof(union ib_gid))) {
+                       ipoib_dbg(
+                               priv,
+                               "%s got PathRec for gid %pI6 while asked for %pI6\n",
+                               dev->name, pathrec->dgid.raw,
+                               path->pathrec.dgid.raw);
+                       memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
+                              sizeof(union ib_gid));
+               }
+
                path->pathrec = *pathrec;
 
                old_ah   = path->ah;