[openib-general] Re: ia64: kernel unaligned access in ipoib
Michael S. Tsirkin
mst at mellanox.co.il
Mon May 29 09:14:05 PDT 2006
Quoting r. Michael S. Tsirkin <mst at mellanox.co.il>:
> Here's a list of places that cast a misaligned address to union ib_gid:
>
> ~>grep -n -e '->ha + 4' *c
>
> ipoib_main.c:507: path = __path_find(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4));
> ipoib_main.c:510: (union ib_gid *) (skb->dst->neighbour->ha + 4));
> ipoib_main.c:560: ipoib_mcast_send(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4), skb);
> ipoib_main.c:776: IPOIB_GID_ARG(*((union ib_gid *) (n->ha +
> 4))));
>
> So, I think the fix will be
> 1. pass gid inside a void * without cast so that the compiler does not assume
> its aligned
> 2. fix IPOIB_GID_FMT/IPOIB_GID_ARG to read the gid byte by byte and not
> by 16 byte chunks
We will be testing the following patch, will let you know later:
its a bit big but the change itself is trivial.
---
Fix misaligned access faults on ia64: never cast a misaligned ha + 4 pointer to
union ib_gid type, pass a void * pointer instead. Note that the cast in
IPOIB_GID_ARG is safe, since we fixed it to only access each byte separately.
Signed-off-by: Jack Morgenstein <jackm at mellanox.co.il>
Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>
Index: openib/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- openib/drivers/infiniband/ulp/ipoib/ipoib_main.c (revision 7541)
+++ openib/drivers/infiniband/ulp/ipoib/ipoib_main.c (working copy)
@@ -190,8 +190,7 @@ static int ipoib_change_mtu(struct net_d
return 0;
}
-static struct ipoib_path *__path_find(struct net_device *dev,
- union ib_gid *gid)
+static struct ipoib_path *__path_find(struct net_device *dev, void *gid)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct rb_node *n = priv->path_tree.rb_node;
@@ -201,7 +200,7 @@ static struct ipoib_path *__path_find(st
while (n) {
path = rb_entry(n, struct ipoib_path, rb_node);
- ret = memcmp(gid->raw, path->pathrec.dgid.raw,
+ ret = memcmp(gid, path->pathrec.dgid.raw,
sizeof (union ib_gid));
if (ret < 0)
@@ -430,8 +429,7 @@ static void path_rec_completion(int stat
}
}
-static struct ipoib_path *path_rec_create(struct net_device *dev,
- union ib_gid *gid)
+static struct ipoib_path *path_rec_create(struct net_device *dev, void *gid)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_path *path;
@@ -446,7 +444,7 @@ static struct ipoib_path *path_rec_creat
INIT_LIST_HEAD(&path->neigh_list);
- memcpy(path->pathrec.dgid.raw, gid->raw, sizeof (union ib_gid));
+ memcpy(path->pathrec.dgid.raw, gid, sizeof (union ib_gid));
path->pathrec.sgid = priv->local_gid;
path->pathrec.pkey = cpu_to_be16(priv->pkey);
path->pathrec.numb_path = 1;
@@ -504,10 +502,9 @@ static void neigh_add_path(struct sk_buf
*/
spin_lock(&priv->lock);
- path = __path_find(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4));
+ path = __path_find(dev, skb->dst->neighbour->ha + 4);
if (!path) {
- path = path_rec_create(dev,
- (union ib_gid *) (skb->dst->neighbour->ha + 4));
+ path = path_rec_create(dev, skb->dst->neighbour->ha + 4);
if (!path)
goto err_path;
@@ -557,7 +554,7 @@ static void ipoib_path_lookup(struct sk_
/* Add in the P_Key for multicasts */
skb->dst->neighbour->ha[8] = (priv->pkey >> 8) & 0xff;
skb->dst->neighbour->ha[9] = priv->pkey & 0xff;
- ipoib_mcast_send(dev, (union ib_gid *) (skb->dst->neighbour->ha + 4), skb);
+ ipoib_mcast_send(dev, skb->dst->neighbour->ha + 4, skb);
}
static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
@@ -572,10 +569,9 @@ static void unicast_arp_send(struct sk_b
*/
spin_lock(&priv->lock);
- path = __path_find(dev, (union ib_gid *) (phdr->hwaddr + 4));
+ path = __path_find(dev, phdr->hwaddr + 4);
if (!path) {
- path = path_rec_create(dev,
- (union ib_gid *) (phdr->hwaddr + 4));
+ path = path_rec_create(dev, phdr->hwaddr + 4);
if (path) {
/* put pseudoheader back on for next time */
skb_push(skb, sizeof *phdr);
@@ -666,7 +662,7 @@ static int ipoib_start_xmit(struct sk_bu
phdr->hwaddr[8] = (priv->pkey >> 8) & 0xff;
phdr->hwaddr[9] = priv->pkey & 0xff;
- ipoib_mcast_send(dev, (union ib_gid *) (phdr->hwaddr + 4), skb);
+ ipoib_mcast_send(dev, phdr->hwaddr + 4, skb);
} else {
/* unicast GID -- should be ARP or RARP reply */
Index: openib/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
===================================================================
--- openib/drivers/infiniband/ulp/ipoib/ipoib_multicast.c (revision 7541)
+++ openib/drivers/infiniband/ulp/ipoib/ipoib_multicast.c (working copy)
@@ -153,7 +153,7 @@ static struct ipoib_mcast *ipoib_mcast_a
return mcast;
}
-static struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, union ib_gid *mgid)
+static struct ipoib_mcast *__ipoib_mcast_find(struct net_device *dev, void *mgid)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct rb_node *n = priv->multicast_tree.rb_node;
@@ -164,7 +164,7 @@ static struct ipoib_mcast *__ipoib_mcast
mcast = rb_entry(n, struct ipoib_mcast, rb_node);
- ret = memcmp(mgid->raw, mcast->mcmember.mgid.raw,
+ ret = memcmp(mgid, mcast->mcmember.mgid.raw,
sizeof (union ib_gid));
if (ret < 0)
n = n->rb_left;
@@ -639,8 +639,7 @@ static int ipoib_mcast_leave(struct net_
return 0;
}
-void ipoib_mcast_send(struct net_device *dev, union ib_gid *mgid,
- struct sk_buff *skb)
+void ipoib_mcast_send(struct net_device *dev, void *mgid, struct sk_buff *skb)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_mcast *mcast;
@@ -675,7 +674,7 @@ void ipoib_mcast_send(struct net_device
}
set_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags);
- mcast->mcmember.mgid = *mgid;
+ memcpy(mcast->mcmember.mgid.raw, mgid, sizeof (union ib_gid));
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
}
Index: openib/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- openib/drivers/infiniband/ulp/ipoib/ipoib.h (revision 7541)
+++ openib/drivers/infiniband/ulp/ipoib/ipoib.h (working copy)
@@ -278,8 +278,7 @@ int ipoib_dev_init(struct net_device *de
void ipoib_dev_cleanup(struct net_device *dev);
void ipoib_mcast_join_task(void *dev_ptr);
-void ipoib_mcast_send(struct net_device *dev, union ib_gid *mgid,
- struct sk_buff *skb);
+void ipoib_mcast_send(struct net_device *dev, void *mgid, struct sk_buff *skb);
void ipoib_mcast_restart_task(void *dev_ptr);
int ipoib_mcast_start_thread(struct net_device *dev);
@@ -375,15 +374,24 @@ extern int ipoib_debug_level;
#endif /* CONFIG_INFINIBAND_IPOIB_DEBUG_DATA */
-#define IPOIB_GID_FMT "%x:%x:%x:%x:%x:%x:%x:%x"
+#define IPOIB_GID_FMT "%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:"
+ "%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x:%2.2x%2.2x"
-#define IPOIB_GID_ARG(gid) be16_to_cpup((__be16 *) ((gid).raw + 0)), \
- be16_to_cpup((__be16 *) ((gid).raw + 2)), \
- be16_to_cpup((__be16 *) ((gid).raw + 4)), \
- be16_to_cpup((__be16 *) ((gid).raw + 6)), \
- be16_to_cpup((__be16 *) ((gid).raw + 8)), \
- be16_to_cpup((__be16 *) ((gid).raw + 10)), \
- be16_to_cpup((__be16 *) ((gid).raw + 12)), \
- be16_to_cpup((__be16 *) ((gid).raw + 14))
+#define IPOIB_GID_ARG(gid) (gid).raw[0], \
+ (gid).raw[1], \
+ (gid).raw[2], \
+ (gid).raw[3], \
+ (gid).raw[4], \
+ (gid).raw[5], \
+ (gid).raw[6], \
+ (gid).raw[7], \
+ (gid).raw[8], \
+ (gid).raw[9], \
+ (gid).raw[10],\
+ (gid).raw[11],\
+ (gid).raw[12],\
+ (gid).raw[13],\
+ (gid).raw[14],\
+ (gid).raw[15]
#endif /* _IPOIB_H */
--
MST
More information about the general
mailing list