On Tue, Nov 14, 2017 at 02:51:53PM +0200, Leon Romanovsky wrote:
> From: Erez Shitrit <erezsh@xxxxxxxxxxxx>
>
> If from some reason the SM responses to path query request with response
> that doesn't contain the exact SGID, ipoib should warn and change that
> part of the response before push it to the path record DB.
> Otherwise, new record will be added to the path record DB with no access
> to them from the ipoib.
>
> Demonstration of the bug is as the follow:
> 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, fe80:0000:0000:0001: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@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 12b7f911f0e5..b173d618c59c 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -775,6 +775,16 @@ static void path_rec_completion(int status,
> spin_lock_irqsave(&priv->lock, flags);
>
> if (!IS_ERR_OR_NULL(ah)) {
> + /* check there is no mismatch from the request */
> + if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
> + sizeof(union ib_gid))) {
> + pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
> + dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
> + /* overwrite the response from the sm before copy to the db */
> + memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
> + sizeof(union ib_gid));
> + }
> +
> path->pathrec = *pathrec;
I applied this, but converted the pr_warn to ipoib_dbg (it is not an
error for the SA to change the DGID), reworded the commit message and
comment:
/*
* 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));
}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
