Commit 546481c2 authored by Erez Shitrit's avatar Erez Shitrit Committed by Doug Ledford

IB/ipoib: Fix memory corruption in ipoib cm mode connect flow

When a new CM connection is being requested, ipoib driver copies data
from the path pointer in the CM/tx object, the path object might be
invalid at the point and memory corruption will happened later when now
the CM driver will try using that data.

The next scenario demonstrates it:
	neigh_add_path --> ipoib_cm_create_tx -->
	queue_work (pointer to path is in the cm/tx struct)
	#while the work is still in the queue,
	#the port goes down and causes the ipoib_flush_paths:
	ipoib_flush_paths --> path_free --> kfree(path)
	#at this point the work scheduled starts.
	ipoib_cm_tx_start --> copy from the (invalid)path pointer:
	(memcpy(&pathrec, &p->path->pathrec, sizeof pathrec);)
	 -> memory corruption.

To fix that the driver now starts the CM/tx connection only if that
specific path exists in the general paths database.
This check is protected with the relevant locks, and uses the gid from
the neigh member in the CM/tx object which is valid according to the ref
count that was taken by the CM/tx.

Fixes: 839fcaba ('IPoIB: Connected mode experimental support')
Signed-off-by: default avatarErez Shitrit <erezsh@mellanox.com>
Signed-off-by: default avatarLeon Romanovsky <leon@kernel.org>
Signed-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 68c6bcdd
...@@ -478,6 +478,7 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb, ...@@ -478,6 +478,7 @@ void ipoib_send(struct net_device *dev, struct sk_buff *skb,
struct ipoib_ah *address, u32 qpn); struct ipoib_ah *address, u32 qpn);
void ipoib_reap_ah(struct work_struct *work); void ipoib_reap_ah(struct work_struct *work);
struct ipoib_path *__path_find(struct net_device *dev, void *gid);
void ipoib_mark_paths_invalid(struct net_device *dev); void ipoib_mark_paths_invalid(struct net_device *dev);
void ipoib_flush_paths(struct net_device *dev); void ipoib_flush_paths(struct net_device *dev);
int ipoib_check_sm_sendonly_fullmember_support(struct ipoib_dev_priv *priv); int ipoib_check_sm_sendonly_fullmember_support(struct ipoib_dev_priv *priv);
......
...@@ -1318,6 +1318,8 @@ void ipoib_cm_destroy_tx(struct ipoib_cm_tx *tx) ...@@ -1318,6 +1318,8 @@ void ipoib_cm_destroy_tx(struct ipoib_cm_tx *tx)
} }
} }
#define QPN_AND_OPTIONS_OFFSET 4
static void ipoib_cm_tx_start(struct work_struct *work) static void ipoib_cm_tx_start(struct work_struct *work)
{ {
struct ipoib_dev_priv *priv = container_of(work, struct ipoib_dev_priv, struct ipoib_dev_priv *priv = container_of(work, struct ipoib_dev_priv,
...@@ -1326,6 +1328,7 @@ static void ipoib_cm_tx_start(struct work_struct *work) ...@@ -1326,6 +1328,7 @@ static void ipoib_cm_tx_start(struct work_struct *work)
struct ipoib_neigh *neigh; struct ipoib_neigh *neigh;
struct ipoib_cm_tx *p; struct ipoib_cm_tx *p;
unsigned long flags; unsigned long flags;
struct ipoib_path *path;
int ret; int ret;
struct ib_sa_path_rec pathrec; struct ib_sa_path_rec pathrec;
...@@ -1338,7 +1341,19 @@ static void ipoib_cm_tx_start(struct work_struct *work) ...@@ -1338,7 +1341,19 @@ static void ipoib_cm_tx_start(struct work_struct *work)
p = list_entry(priv->cm.start_list.next, typeof(*p), list); p = list_entry(priv->cm.start_list.next, typeof(*p), list);
list_del_init(&p->list); list_del_init(&p->list);
neigh = p->neigh; neigh = p->neigh;
qpn = IPOIB_QPN(neigh->daddr); qpn = IPOIB_QPN(neigh->daddr);
/*
* As long as the search is with these 2 locks,
* path existence indicates its validity.
*/
path = __path_find(dev, neigh->daddr + QPN_AND_OPTIONS_OFFSET);
if (!path) {
pr_info("%s ignore not valid path %pI6\n",
__func__,
neigh->daddr + QPN_AND_OPTIONS_OFFSET);
goto free_neigh;
}
memcpy(&pathrec, &p->path->pathrec, sizeof pathrec); memcpy(&pathrec, &p->path->pathrec, sizeof pathrec);
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock_irqrestore(&priv->lock, flags);
...@@ -1350,6 +1365,7 @@ static void ipoib_cm_tx_start(struct work_struct *work) ...@@ -1350,6 +1365,7 @@ static void ipoib_cm_tx_start(struct work_struct *work)
spin_lock_irqsave(&priv->lock, flags); spin_lock_irqsave(&priv->lock, flags);
if (ret) { if (ret) {
free_neigh:
neigh = p->neigh; neigh = p->neigh;
if (neigh) { if (neigh) {
neigh->cm = NULL; neigh->cm = NULL;
......
...@@ -485,7 +485,7 @@ int ipoib_set_mode(struct net_device *dev, const char *buf) ...@@ -485,7 +485,7 @@ int ipoib_set_mode(struct net_device *dev, const char *buf)
return -EINVAL; return -EINVAL;
} }
static struct ipoib_path *__path_find(struct net_device *dev, void *gid) struct ipoib_path *__path_find(struct net_device *dev, void *gid)
{ {
struct ipoib_dev_priv *priv = netdev_priv(dev); struct ipoib_dev_priv *priv = netdev_priv(dev);
struct rb_node *n = priv->path_tree.rb_node; struct rb_node *n = priv->path_tree.rb_node;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment