[PATCH AUTOSEL for 4.15 41/78] IB/ipoib: Avoid memory leak if the SA returns a different DGID

2018-03-07 Thread Sasha Levin
From: Erez Shitrit 

[ Upstream commit 439000892ee17a9c92f1e4297818790ef8bb4ced ]

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::::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::::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 
Signed-off-by: Leon Romanovsky 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Sasha Levin 
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index c35c28015f8e..160c5d9bca4c 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -775,6 +775,22 @@ static void path_rec_completion(int status,
spin_lock_irqsave(>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;
-- 
2.14.1


[PATCH AUTOSEL for 4.15 41/78] IB/ipoib: Avoid memory leak if the SA returns a different DGID

2018-03-07 Thread Sasha Levin
From: Erez Shitrit 

[ Upstream commit 439000892ee17a9c92f1e4297818790ef8bb4ced ]

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::::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::::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 
Signed-off-by: Leon Romanovsky 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Sasha Levin 
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index c35c28015f8e..160c5d9bca4c 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -775,6 +775,22 @@ static void path_rec_completion(int status,
spin_lock_irqsave(>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;
-- 
2.14.1