[BNEP] Fix compat BNEPGETCONNLIST ioctl.

[BNEP] Fix compat BNEPGETCONNLIST ioctl.

[David Woodhouse]

We were making no attempt to deal with the fact that a structure with a
uint32_t followed by a pointer is going to be _different_ for 32-bit and
64-bit userspace. Any 32-bit process trying to use BNEPGETCONNLIST will
be failing with -EFAULT if it's lucky; suffering from having the
connection list dumped at a random address if it's not.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/net/bluetooth/bnep/sock.c b/net/bluetooth/bnep/sock.c
index 28c5583..0ef2783 100644
--- a/net/bluetooth/bnep/sock.c
+++ b/net/bluetooth/bnep/sock.c
@@ -43,6 +43,7 @@ #include <linux/socket.h>
 #include <linux/ioctl.h>
 #include <linux/file.h>
 #include <linux/init.h>
+#include <linux/compat.h>
 #include <net/sock.h>
 
 #include <asm/system.h>
@@ -146,11 +147,44 @@ static int bnep_sock_ioctl(struct socket
 	return 0;
 }
 
+#ifdef CONFIG_COMPAT
+static int bnep_sock_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+
+	if (cmd == BNEPGETCONNLIST) {
+		struct bnep_connlist_req cl;
+		uint32_t uci;
+		int err;
+
+		if (get_user(cl.cnum, (uint32_t __user *)arg) ||
+		    get_user(uci, (u32 __user *)(arg+4)))
+			return -EFAULT;
+
+		cl.ci = compat_ptr(uci);
+
+		if (cl.cnum <= 0)
+			return -EINVAL;
+	
+		err = bnep_get_connlist(&cl);
+
+		if (!err && put_user(cl.cnum, (uint32_t __user *)arg))
+			err = -EFAULT;
+
+		return err;
+	}
+
+	return bnep_sock_ioctl(sock, cmd, arg);
+}
+#endif
+
 static const struct proto_ops bnep_sock_ops = {
 	.family     = PF_BLUETOOTH,
 	.owner      = THIS_MODULE,
 	.release    = bnep_sock_release,
 	.ioctl      = bnep_sock_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = bnep_sock_compat_ioctl,
+#endif
 	.bind       = sock_no_bind,
 	.getname    = sock_no_getname,
 	.sendmsg    = sock_no_sendmsg,

-- 
dwmw2

Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl.

[Marcel Holtmann]

Hi David,

> We were making no attempt to deal with the fact that a structure with a
> uint32_t followed by a pointer is going to be _different_ for 32-bit and
> 64-bit userspace. Any 32-bit process trying to use BNEPGETCONNLIST will
> be failing with -EFAULT if it's lucky; suffering from having the
> connection list dumped at a random address if it's not.

it seems that HIDP and CMTP will have the same problem.

Regards

Marcel

Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl.

[David Woodhouse]

On Mon, 2006-09-18 at 12:38 +0200, Marcel Holtmann wrote:
> Hi David,
> 
> > We were making no attempt to deal with the fact that a structure with a
> > uint32_t followed by a pointer is going to be _different_ for 32-bit and
> > 64-bit userspace. Any 32-bit process trying to use BNEPGETCONNLIST will
> > be failing with -EFAULT if it's lucky; suffering from having the
> > connection list dumped at a random address if it's not.
> 
> it seems that HIDP and CMTP will have the same problem.

Indeed they do. This patch fixes 'hidd -l'... although HIDP mouse
movement doesn't seem to be appearing in /dev/input/mice on my G5, while
the 'hcidump' output looks sane enough while I move it.

-----
[HIDP] Fix compat HIDPGETCONNLIST ioctl.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/net/bluetooth/hidp/sock.c b/net/bluetooth/hidp/sock.c
index 099646e..af5a21c 100644
--- a/net/bluetooth/hidp/sock.c
+++ b/net/bluetooth/hidp/sock.c
@@ -35,6 +35,7 @@ #include <linux/socket.h>
 #include <linux/ioctl.h>
 #include <linux/file.h>
 #include <linux/init.h>
+#include <linux/compat.h>
 #include <net/sock.h>
 
 #include "hidp.h"
@@ -143,11 +144,42 @@ static int hidp_sock_ioctl(struct socket
 	return -EINVAL;
 }
 
+#ifdef CONFIG_COMPAT
+static int hidp_sock_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+	if (cmd == HIDPGETCONNLIST) {
+		struct hidp_connlist_req cl;
+		uint32_t uci;
+		int err;
+
+		if (get_user(cl.cnum, (uint32_t __user *)arg) ||
+		    get_user(uci, (u32 __user *)(arg+4)))
+			return -EFAULT;
+
+		cl.ci = compat_ptr(uci);
+
+		if (cl.cnum <= 0)
+			return -EINVAL;
+
+		err = hidp_get_connlist(&cl);
+
+		if (!err && put_user(cl.cnum, (uint32_t __user *)arg))
+			err = -EFAULT;
+
+		return err;
+	}
+	return hidp_sock_ioctl(sock, cmd, arg);
+}
+#endif
+
 static const struct proto_ops hidp_sock_ops = {
 	.family		= PF_BLUETOOTH,
 	.owner		= THIS_MODULE,
 	.release	= hidp_sock_release,
 	.ioctl		= hidp_sock_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= hidp_sock_compat_ioctl,
+#endif
 	.bind		= sock_no_bind,
 	.getname	= sock_no_getname,
 	.sendmsg	= sock_no_sendmsg,

-- 
dwmw2

Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl.

[David Woodhouse]

On Mon, 2006-09-18 at 14:25 +0100, David Woodhouse wrote:
> although HIDP mouse movement doesn't seem to be appearing
> in /dev/input/mice on my G5, while the 'hcidump' output looks sane
> enough while I move it. 

Ew, that's because struct hidp_connadd_req is similarly buggered for
compat. Replacement HIDP patch to fix both at once... I didn't miss
anywhere where we actually change the hidp_connadd_req structure during
the call, did I?

-----
[HIDP] Fix compat HIDPGETCONNLIST and HIDPCONNADD ioctls.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/net/bluetooth/hidp/sock.c b/net/bluetooth/hidp/sock.c
index 099646e..e01fdc5 100644
--- a/net/bluetooth/hidp/sock.c
+++ b/net/bluetooth/hidp/sock.c
@@ -35,6 +35,7 @@ #include <linux/socket.h>
 #include <linux/ioctl.h>
 #include <linux/file.h>
 #include <linux/init.h>
+#include <linux/compat.h>
 #include <net/sock.h>
 
 #include "hidp.h"
@@ -143,11 +144,87 @@ static int hidp_sock_ioctl(struct socket
 	return -EINVAL;
 }
 
+#ifdef CONFIG_COMPAT
+struct compat_hidp_connadd_req {
+	int   ctrl_sock;	// Connected control socket
+	int   intr_sock;	// Connteted interrupt socket
+	__u16 parser;
+	__u16 rd_size;
+	compat_uptr_t rd_data;
+	__u8  country;
+	__u8  subclass;
+	__u16 vendor;
+	__u16 product;
+	__u16 version;
+	__u32 flags;
+	__u32 idle_to;
+	char  name[128];
+};
+
+static int hidp_sock_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+	if (cmd == HIDPGETCONNLIST) {
+		struct hidp_connlist_req cl;
+		uint32_t uci;
+		int err;
+
+		if (get_user(cl.cnum, (uint32_t __user *)arg) ||
+		    get_user(uci, (u32 __user *)(arg+4)))
+			return -EFAULT;
+
+		cl.ci = compat_ptr(uci);
+
+		if (cl.cnum <= 0)
+			return -EINVAL;
+
+		err = hidp_get_connlist(&cl);
+
+		if (!err && put_user(cl.cnum, (uint32_t __user *)arg))
+			err = -EFAULT;
+
+		return err;
+	} else if (cmd == HIDPCONNADD) {
+		struct compat_hidp_connadd_req ca;
+		struct hidp_connadd_req __user *uca;
+
+		uca = compat_alloc_user_space(sizeof(*uca));
+
+		if (copy_from_user(&ca, (void *)arg, sizeof(ca)))
+			return -EFAULT;
+
+		if (put_user(ca.ctrl_sock, &uca->ctrl_sock)
+		    || put_user(ca.intr_sock, &uca->intr_sock)
+		    || put_user(ca.parser, &uca->parser)
+		    || put_user(ca.rd_size, &uca->parser)
+		    || put_user(compat_ptr(ca.rd_data), &uca->rd_data)
+		    || put_user(ca.country, &uca->country)
+		    || put_user(ca.subclass, &uca->subclass)
+		    || put_user(ca.vendor, &uca->vendor)
+		    || put_user(ca.product, &uca->product)
+		    || put_user(ca.version, &uca->version)
+		    || put_user(ca.flags, &uca->flags)
+		    || put_user(ca.idle_to, &uca->idle_to)
+		    || copy_to_user(&uca->name[0], &ca.name[0], 128))
+			return -EFAULT;
+		
+		arg = (unsigned long)uca;
+		/* Fall through. We don't actually write back any _changes_
+		   to the structure anyway, so there's no need to copy back
+		   into the original compat version */
+	}
+
+	return hidp_sock_ioctl(sock, cmd, arg);
+}
+#endif
+
 static const struct proto_ops hidp_sock_ops = {
 	.family		= PF_BLUETOOTH,
 	.owner		= THIS_MODULE,
 	.release	= hidp_sock_release,
 	.ioctl		= hidp_sock_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= hidp_sock_compat_ioctl,
+#endif
 	.bind		= sock_no_bind,
 	.getname	= sock_no_getname,
 	.sendmsg	= sock_no_sendmsg,


-- 
dwmw2

Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl.

[David Woodhouse]

On Mon, 2006-09-18 at 12:38 +0200, Marcel Holtmann wrote:
> it seems that HIDP and CMTP will have the same problem.

Finally, the CMTP version... this one is untested.

----
[CMTP] Fix compat CMTPGETCONNLIST ioctl

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
index 10ad7fd..68e1290 100644
--- a/net/bluetooth/cmtp/sock.c
+++ b/net/bluetooth/cmtp/sock.c
@@ -34,6 +34,7 @@ #include <linux/skbuff.h>
 #include <linux/socket.h>
 #include <linux/ioctl.h>
 #include <linux/file.h>
+#include <linux/compat.h>
 #include <net/sock.h>
 
 #include <linux/isdn/capilli.h>
@@ -137,11 +138,44 @@ static int cmtp_sock_ioctl(struct socket
 	return -EINVAL;
 }
 
+#ifdef CONFIG_COMPAT
+static int cmtp_sock_compat_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+
+	if (cmd == CMTPGETCONNLIST) {
+		struct cmtp_connlist_req cl;
+		uint32_t uci;
+		int err;
+
+		if (get_user(cl.cnum, (uint32_t __user *)arg) ||
+		    get_user(uci, (u32 __user *)(arg+4)))
+			return -EFAULT;
+
+		cl.ci = compat_ptr(uci);
+
+		if (cl.cnum <= 0)
+			return -EINVAL;
+	
+		err = cmtp_get_connlist(&cl);
+
+		if (!err && put_user(cl.cnum, (uint32_t __user *)arg))
+			err = -EFAULT;
+
+		return err;
+	}
+
+	return cmtp_sock_ioctl(sock, cmd, arg);
+}
+#endif
+
 static const struct proto_ops cmtp_sock_ops = {
 	.family		= PF_BLUETOOTH,
 	.owner		= THIS_MODULE,
 	.release	= cmtp_sock_release,
 	.ioctl		= cmtp_sock_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= cmtp_sock_compat_ioctl,
+#endif
 	.bind		= sock_no_bind,
 	.getname	= sock_no_getname,
 	.sendmsg	= sock_no_sendmsg,

-- 
dwmw2

Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl.

[Marcel Holtmann]

Hi David,

> > although HIDP mouse movement doesn't seem to be appearing
> > in /dev/input/mice on my G5, while the 'hcidump' output looks sane
> > enough while I move it. 
> 
> Ew, that's because struct hidp_connadd_req is similarly buggered for
> compat. Replacement HIDP patch to fix both at once... I didn't miss
> anywhere where we actually change the hidp_connadd_req structure during
> the call, did I?

that looks ugly, but I assume there is no other way to solve this
problem. I will go over all three patches and wrap them up nicely.

Linus, will you accept these for inclusion before 2.6.18 final?

Regards

Marcel