Commit a394f83b authored by Roland Dreier's avatar Roland Dreier

IB/umad: Fix bit ordering and 32-on-64 problems on big endian systems

The declaration of struct ib_user_mad_reg_req.method_mask[] exported
to userspace was an array of __u32, but the kernel internally treated
it as a bitmap made up of longs.  This makes a difference for 64-bit
big-endian kernels, where numbering the bits in an array of__u32 gives:

    |31.....0|63....31|95....64|127...96|

while numbering the bits in an array of longs gives:

    |63..............0|127............64|

64-bit userspace can handle this by just treating method_mask[] as an
array of longs, but 32-bit userspace is really stuck: the meaning of
the bits in method_mask[] depends on whether the kernel is 32-bit or
64-bit, and there's no sane way for userspace to know that.

Fix this by updating <rdma/ib_user_mad.h> to make it clear that
method_mask[] is an array of longs, and using a compat_ioctl method to
convert to an array of 64-bit longs to handle the 32-on-64 problem.
This fixes the interface description to match existing behavior (so
working binaries continue to work) in almost all situations, and gives
consistent semantics in the case of 32-bit userspace that can run on
either a 32-bit or 64-bit kernel, so that the same binary can work for
both 32-on-32 and 32-on-64 systems.
Signed-off-by: default avatarRoland Dreier <rolandd@cisco.com>
parent 2be8e3ee
...@@ -44,6 +44,7 @@ ...@@ -44,6 +44,7 @@
#include <linux/poll.h> #include <linux/poll.h>
#include <linux/rwsem.h> #include <linux/rwsem.h>
#include <linux/kref.h> #include <linux/kref.h>
#include <linux/compat.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <asm/semaphore.h> #include <asm/semaphore.h>
...@@ -607,7 +608,8 @@ static unsigned int ib_umad_poll(struct file *filp, struct poll_table_struct *wa ...@@ -607,7 +608,8 @@ static unsigned int ib_umad_poll(struct file *filp, struct poll_table_struct *wa
return mask; return mask;
} }
static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg) static int ib_umad_reg_agent(struct ib_umad_file *file, void __user *arg,
int compat_method_mask)
{ {
struct ib_user_mad_reg_req ureq; struct ib_user_mad_reg_req ureq;
struct ib_mad_reg_req req; struct ib_mad_reg_req req;
...@@ -622,7 +624,7 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg) ...@@ -622,7 +624,7 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg)
goto out; goto out;
} }
if (copy_from_user(&ureq, (void __user *) arg, sizeof ureq)) { if (copy_from_user(&ureq, arg, sizeof ureq)) {
ret = -EFAULT; ret = -EFAULT;
goto out; goto out;
} }
...@@ -643,8 +645,18 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg) ...@@ -643,8 +645,18 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg)
if (ureq.mgmt_class) { if (ureq.mgmt_class) {
req.mgmt_class = ureq.mgmt_class; req.mgmt_class = ureq.mgmt_class;
req.mgmt_class_version = ureq.mgmt_class_version; req.mgmt_class_version = ureq.mgmt_class_version;
memcpy(req.method_mask, ureq.method_mask, sizeof req.method_mask);
memcpy(req.oui, ureq.oui, sizeof req.oui); memcpy(req.oui, ureq.oui, sizeof req.oui);
if (compat_method_mask) {
u32 *umm = (u32 *) ureq.method_mask;
int i;
for (i = 0; i < BITS_TO_LONGS(IB_MGMT_MAX_METHODS); ++i)
req.method_mask[i] =
umm[i * 2] | ((u64) umm[i * 2 + 1] << 32);
} else
memcpy(req.method_mask, ureq.method_mask,
sizeof req.method_mask);
} }
agent = ib_register_mad_agent(file->port->ib_dev, file->port->port_num, agent = ib_register_mad_agent(file->port->ib_dev, file->port->port_num,
...@@ -682,13 +694,13 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg) ...@@ -682,13 +694,13 @@ static int ib_umad_reg_agent(struct ib_umad_file *file, unsigned long arg)
return ret; return ret;
} }
static int ib_umad_unreg_agent(struct ib_umad_file *file, unsigned long arg) static int ib_umad_unreg_agent(struct ib_umad_file *file, u32 __user *arg)
{ {
struct ib_mad_agent *agent = NULL; struct ib_mad_agent *agent = NULL;
u32 id; u32 id;
int ret = 0; int ret = 0;
if (get_user(id, (u32 __user *) arg)) if (get_user(id, arg))
return -EFAULT; return -EFAULT;
down_write(&file->port->mutex); down_write(&file->port->mutex);
...@@ -729,15 +741,32 @@ static long ib_umad_ioctl(struct file *filp, unsigned int cmd, ...@@ -729,15 +741,32 @@ static long ib_umad_ioctl(struct file *filp, unsigned int cmd,
{ {
switch (cmd) { switch (cmd) {
case IB_USER_MAD_REGISTER_AGENT: case IB_USER_MAD_REGISTER_AGENT:
return ib_umad_reg_agent(filp->private_data, arg); return ib_umad_reg_agent(filp->private_data, (void __user *) arg, 0);
case IB_USER_MAD_UNREGISTER_AGENT:
return ib_umad_unreg_agent(filp->private_data, (__u32 __user *) arg);
case IB_USER_MAD_ENABLE_PKEY:
return ib_umad_enable_pkey(filp->private_data);
default:
return -ENOIOCTLCMD;
}
}
#ifdef CONFIG_COMPAT
static long ib_umad_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
switch (cmd) {
case IB_USER_MAD_REGISTER_AGENT:
return ib_umad_reg_agent(filp->private_data, compat_ptr(arg), 1);
case IB_USER_MAD_UNREGISTER_AGENT: case IB_USER_MAD_UNREGISTER_AGENT:
return ib_umad_unreg_agent(filp->private_data, arg); return ib_umad_unreg_agent(filp->private_data, compat_ptr(arg));
case IB_USER_MAD_ENABLE_PKEY: case IB_USER_MAD_ENABLE_PKEY:
return ib_umad_enable_pkey(filp->private_data); return ib_umad_enable_pkey(filp->private_data);
default: default:
return -ENOIOCTLCMD; return -ENOIOCTLCMD;
} }
} }
#endif
static int ib_umad_open(struct inode *inode, struct file *filp) static int ib_umad_open(struct inode *inode, struct file *filp)
{ {
...@@ -826,7 +855,9 @@ static const struct file_operations umad_fops = { ...@@ -826,7 +855,9 @@ static const struct file_operations umad_fops = {
.write = ib_umad_write, .write = ib_umad_write,
.poll = ib_umad_poll, .poll = ib_umad_poll,
.unlocked_ioctl = ib_umad_ioctl, .unlocked_ioctl = ib_umad_ioctl,
.compat_ioctl = ib_umad_ioctl, #ifdef CONFIG_COMPAT
.compat_ioctl = ib_umad_compat_ioctl,
#endif
.open = ib_umad_open, .open = ib_umad_open,
.release = ib_umad_close .release = ib_umad_close
}; };
......
...@@ -147,6 +147,26 @@ struct ib_user_mad { ...@@ -147,6 +147,26 @@ struct ib_user_mad {
__u64 data[0]; __u64 data[0];
}; };
/*
* Earlier versions of this interface definition declared the
* method_mask[] member as an array of __u32 but treated it as a
* bitmap made up of longs in the kernel. This ambiguity meant that
* 32-bit big-endian applications that can run on both 32-bit and
* 64-bit kernels had no consistent ABI to rely on, and 64-bit
* big-endian applications that treated method_mask as being made up
* of 32-bit words would have their bitmap misinterpreted.
*
* To clear up this confusion, we change the declaration of
* method_mask[] to use unsigned long and handle the conversion from
* 32-bit userspace to 64-bit kernel for big-endian systems in the
* compat_ioctl method. Unfortunately, to keep the structure layout
* the same, we need the method_mask[] array to be aligned only to 4
* bytes even when long is 64 bits, which forces us into this ugly
* typedef.
*/
typedef unsigned long __attribute__((aligned(4))) packed_ulong;
#define IB_USER_MAD_LONGS_PER_METHOD_MASK (128 / (8 * sizeof (long)))
/** /**
* ib_user_mad_reg_req - MAD registration request * ib_user_mad_reg_req - MAD registration request
* @id - Set by the kernel; used to identify agent in future requests. * @id - Set by the kernel; used to identify agent in future requests.
...@@ -165,7 +185,7 @@ struct ib_user_mad { ...@@ -165,7 +185,7 @@ struct ib_user_mad {
*/ */
struct ib_user_mad_reg_req { struct ib_user_mad_reg_req {
__u32 id; __u32 id;
__u32 method_mask[4]; packed_ulong method_mask[IB_USER_MAD_LONGS_PER_METHOD_MASK];
__u8 qpn; __u8 qpn;
__u8 mgmt_class; __u8 mgmt_class;
__u8 mgmt_class_version; __u8 mgmt_class_version;
......
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