* [BNEP] Fix compat BNEPGETCONNLIST ioctl. @ 2006-09-18 10:05 David Woodhouse 2006-09-18 10:38 ` Marcel Holtmann 0 siblings, 1 reply; 6+ messages in thread From: David Woodhouse @ 2006-09-18 10:05 UTC (permalink / raw) To: marcel, torvalds, akpm; +Cc: netdev 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl. 2006-09-18 10:05 [BNEP] Fix compat BNEPGETCONNLIST ioctl David Woodhouse @ 2006-09-18 10:38 ` Marcel Holtmann 2006-09-18 13:24 ` David Woodhouse 2006-09-18 14:28 ` David Woodhouse 0 siblings, 2 replies; 6+ messages in thread From: Marcel Holtmann @ 2006-09-18 10:38 UTC (permalink / raw) To: David Woodhouse; +Cc: torvalds, akpm, netdev 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl. 2006-09-18 10:38 ` Marcel Holtmann @ 2006-09-18 13:24 ` David Woodhouse 2006-09-18 14:19 ` David Woodhouse 2006-09-18 14:28 ` David Woodhouse 1 sibling, 1 reply; 6+ messages in thread From: David Woodhouse @ 2006-09-18 13:24 UTC (permalink / raw) To: Marcel Holtmann; +Cc: torvalds, akpm, netdev 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl. 2006-09-18 13:24 ` David Woodhouse @ 2006-09-18 14:19 ` David Woodhouse 2006-09-18 15:09 ` Marcel Holtmann 0 siblings, 1 reply; 6+ messages in thread From: David Woodhouse @ 2006-09-18 14:19 UTC (permalink / raw) To: Marcel Holtmann; +Cc: torvalds, akpm, netdev 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl. 2006-09-18 14:19 ` David Woodhouse @ 2006-09-18 15:09 ` Marcel Holtmann 0 siblings, 0 replies; 6+ messages in thread From: Marcel Holtmann @ 2006-09-18 15:09 UTC (permalink / raw) To: David Woodhouse; +Cc: torvalds, akpm, netdev 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BNEP] Fix compat BNEPGETCONNLIST ioctl. 2006-09-18 10:38 ` Marcel Holtmann 2006-09-18 13:24 ` David Woodhouse @ 2006-09-18 14:28 ` David Woodhouse 1 sibling, 0 replies; 6+ messages in thread From: David Woodhouse @ 2006-09-18 14:28 UTC (permalink / raw) To: Marcel Holtmann; +Cc: torvalds, akpm, netdev 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-09-18 15:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-09-18 10:05 [BNEP] Fix compat BNEPGETCONNLIST ioctl David Woodhouse 2006-09-18 10:38 ` Marcel Holtmann 2006-09-18 13:24 ` David Woodhouse 2006-09-18 14:19 ` David Woodhouse 2006-09-18 15:09 ` Marcel Holtmann 2006-09-18 14:28 ` David Woodhouse
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).