[PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[Shaun Pereira]

Hi all,
The attached patch is a follow up to a post made earlier to this site
with regard to 32 bit (socket layer) ioctl emulation for 64 bit kernels.

I needed to implement 32 bit userland ioctl support for modular (x.25)
socket ioctls in a 64 bit kernel. With the removal of the
register_ioctl32_conversion() function from the kernel, one of the
suggestions made by Andi was "to just extend the socket code to add a
compat_ioctl vector to the socket options"

With Arnd's help (see previous mails with subject = 32 bit (socket
layer) ioctl emulation for 64 bit kernels) I have prepared the following
patchand tested with with x25 over tcp on a x26_64 kernel. 

Since we are interested in ioctl's from userspace I have not added the 
.compat_ioctl function pointer to struct net_device. The assumption
being once the userspace data has reached the kernel via the socket api,
if the socket layer protocol knows how to handle the data, it will
prepare it for the device. 

Am not too sure whether struct proto requires modification. Since it is 
allocated dynamically in the protocol layer I have left it alone;no
compat_ioctl. Also it seems like the socket layer would know how to
"ioctl" the transport layer, userspace does not need to know about
this? 

But if any of this is incorrect and needs to be changed please advise 
and I will make the changes accordingly. If this patch is accepted I 
would be in a position to submit a patch for x25 (32 bit userspace 
for 64 bit kernel). 

Many thanks for your help
Regards
Shaun


diff -uprN -X dontdiff linux-2.6.15-vanilla/include/linux/net.h
linux-2.6.15/include/linux/net.h
--- linux-2.6.15-vanilla/include/linux/net.h	2006-01-03
14:21:10.000000000 +1100
+++ linux-2.6.15/include/linux/net.h	2006-01-09 15:59:49.000000000 +1100
@@ -143,6 +143,10 @@ struct proto_ops {
 				      struct poll_table_struct *wait);
 	int		(*ioctl)     (struct socket *sock, unsigned int cmd,
 				      unsigned long arg);
+#ifdef CONFIG_COMPAT
+	int	 	(*compat_ioctl) (struct socket *sock, unsigned int cmd,
+				      unsigned long arg);
+#endif
 	int		(*listen)    (struct socket *sock, int len);
 	int		(*shutdown)  (struct socket *sock, int flags);
 	int		(*setsockopt)(struct socket *sock, int level,
@@ -205,6 +209,7 @@ extern int   	     kernel_recvmsg(struct
 #ifndef CONFIG_SMP
 #define SOCKOPS_WRAPPED(name) name
 #define SOCKOPS_WRAP(name, fam)
+#define SOCKOPS_COMPAT_WRAP(name, fam)
 #else
 
 #define SOCKOPS_WRAPPED(name) __unlocked_##name
@@ -279,6 +284,60 @@ static struct proto_ops name##_ops = {		
 	.recvmsg	= __lock_##name##_recvmsg,	\
 	.mmap		= __lock_##name##_mmap,		\
 };
+
+#define SOCKOPS_COMPAT_WRAP(name, fam)					\
+SOCKCALL_WRAP(name, release, (struct socket *sock), (sock))	\
+SOCKCALL_WRAP(name, bind, (struct socket *sock, struct sockaddr *uaddr,
int addr_len), \
+	      (sock, uaddr, addr_len))				\
+SOCKCALL_WRAP(name, connect, (struct socket *sock, struct sockaddr *
uaddr, \
+			      int addr_len, int flags), 	\
+	      (sock, uaddr, addr_len, flags))			\
+SOCKCALL_WRAP(name, socketpair, (struct socket *sock1, struct socket
*sock2), \
+	      (sock1, sock2))					\
+SOCKCALL_WRAP(name, accept, (struct socket *sock, struct socket
*newsock, \
+			 int flags), (sock, newsock, flags)) \
+SOCKCALL_WRAP(name, getname, (struct socket *sock, struct sockaddr
*uaddr, \
+			 int *addr_len, int peer), (sock, uaddr, addr_len, peer)) \
+SOCKCALL_UWRAP(name, poll, (struct file *file, struct socket *sock,
struct poll_table_struct *wait), \
+	      (file, sock, wait)) \
+SOCKCALL_WRAP(name, ioctl, (struct socket *sock, unsigned int cmd, \
+			 unsigned long arg), (sock, cmd, arg)) \
+SOCKCALL_WRAP(name, compat_ioctl, (struct socket *sock, unsigned int
cmd, \
+			 unsigned long arg), (sock, cmd, arg)) \
+SOCKCALL_WRAP(name, listen, (struct socket *sock, int len), (sock,
len)) \
+SOCKCALL_WRAP(name, shutdown, (struct socket *sock, int flags), (sock,
flags)) \
+SOCKCALL_WRAP(name, setsockopt, (struct socket *sock, int level, int
optname, \
+			 char __user *optval, int optlen), (sock, level, optname, optval,
optlen)) \
+SOCKCALL_WRAP(name, getsockopt, (struct socket *sock, int level, int
optname, \
+			 char __user *optval, int __user *optlen), (sock, level, optname,
optval, optlen)) \
+SOCKCALL_WRAP(name, sendmsg, (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t len), \
+	      (iocb, sock, m, len)) \
+SOCKCALL_WRAP(name, recvmsg, (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t len, int flags), \
+	      (iocb, sock, m, len, flags)) \
+SOCKCALL_WRAP(name, mmap, (struct file *file, struct socket *sock,
struct vm_area_struct *vma), \
+	      (file, sock, vma)) \
+	      \
+static struct proto_ops name##_ops = {			\
+	.family		= fam,				\
+	.owner		= THIS_MODULE,			\
+	.release	= __lock_##name##_release,	\
+	.bind		= __lock_##name##_bind,		\
+	.connect	= __lock_##name##_connect,	\
+	.socketpair	= __lock_##name##_socketpair,	\
+	.accept		= __lock_##name##_accept,	\
+	.getname	= __lock_##name##_getname,	\
+	.poll		= __lock_##name##_poll,		\
+	.ioctl		= __lock_##name##_ioctl,	\
+	.compat_ioctl	= __lock_##name##_compat_ioctl,	\
+	.listen		= __lock_##name##_listen,	\
+	.shutdown	= __lock_##name##_shutdown,	\
+	.setsockopt	= __lock_##name##_setsockopt,	\
+	.getsockopt	= __lock_##name##_getsockopt,	\
+	.sendmsg	= __lock_##name##_sendmsg,	\
+	.recvmsg	= __lock_##name##_recvmsg,	\
+	.mmap		= __lock_##name##_mmap,		\
+};
+
 #endif
 
 #define MODULE_ALIAS_NETPROTO(proto) \
diff -uprN -X dontdiff linux-2.6.15-vanilla/net/socket.c
linux-2.6.15/net/socket.c
--- linux-2.6.15-vanilla/net/socket.c	2006-01-03 14:21:10.000000000
+1100
+++ linux-2.6.15/net/socket.c	2006-01-09 15:59:49.000000000 +1100
@@ -109,6 +109,10 @@ static unsigned int sock_poll(struct fil
 			      struct poll_table_struct *wait);
 static long sock_ioctl(struct file *file,
 		      unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+static long compat_sock_ioctl(struct file *file,
+		      unsigned int cmd, unsigned long arg);
+#endif
 static int sock_fasync(int fd, struct file *filp, int on);
 static ssize_t sock_readv(struct file *file, const struct iovec
*vector,
 			  unsigned long count, loff_t *ppos);
@@ -130,6 +134,9 @@ static struct file_operations socket_fil
 	.aio_write =	sock_aio_write,
 	.poll =		sock_poll,
 	.unlocked_ioctl = sock_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = compat_sock_ioctl,
+#endif
 	.mmap =		sock_mmap,
 	.open =		sock_no_open,	/* special open code to disallow open via /proc
*/
 	.release =	sock_close,
@@ -2084,6 +2091,20 @@ void socket_seq_show(struct seq_file *se
 }
 #endif /* CONFIG_PROC_FS */
 
+#ifdef CONFIG_COMPAT
+static long compat_sock_ioctl(struct file *file, unsigned cmd, unsigned
long arg)
+{
+	struct socket *sock;
+	sock = file->private_data;
+
+	int ret = -ENOIOCTLCMD;
+	if(sock->ops->compat_ioctl) {
+		ret = sock->ops->compat_ioctl(sock,cmd,arg);
+	}
+	return ret;
+}
+#endif
+
 /* ABI emulation layers need these two */
 EXPORT_SYMBOL(move_addr_to_kernel);
 EXPORT_SYMBOL(move_addr_to_user);

Re: [PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[Arnd Bergmann]

On Monday 09 January 2006 06:46, Shaun Pereira wrote:
> Hi all,
> The attached patch is a follow up to a post made earlier to this site
> with regard to 32 bit (socket layer) ioctl emulation for 64 bit kernels.

Ok, cool. Note that I also posted a longer series of patches that does
this and much more. Unfortunately, I have suspended working on it for
now, so it's probably better to first get your stuff in.

> I needed to implement 32 bit userland ioctl support for modular (x.25)
> socket ioctls in a 64 bit kernel. With the removal of the
> register_ioctl32_conversion() function from the kernel, one of the
> suggestions made by Andi was "to just extend the socket code to add a
> compat_ioctl vector to the socket options"
> 
> With Arnd's help (see previous mails with subject = 32 bit (socket
> layer) ioctl emulation for 64 bit kernels) I have prepared the following
> patchand tested with with x25 over tcp on a x26_64 kernel. 
> 
> Since we are interested in ioctl's from userspace I have not added the 
> .compat_ioctl function pointer to struct net_device. The assumption
> being once the userspace data has reached the kernel via the socket api,
> if the socket layer protocol knows how to handle the data, it will
> prepare it for the device.

I think we need to have it in the long run, but if you don't need it
for x25, then it's not your call to implement net_device->compat_ioctl.
I've been thinking about how to get it right before, but did not
reach a proper conclusion, since dev_ioctl is called in so many places
that would all need to be changed for this.

> Am not too sure whether struct proto requires modification. Since it is 
> allocated dynamically in the protocol layer I have left it alone;no
> compat_ioctl. Also it seems like the socket layer would know how to
> "ioctl" the transport layer, userspace does not need to know about
> this? 

The proto ioctls are all forwarded from sock ioctls, so in theory
it would be needed. 

> But if any of this is incorrect and needs to be changed please advise 
> and I will make the changes accordingly. If this patch is accepted I 
> would be in a position to submit a patch for x25 (32 bit userspace 
> for 64 bit kernel). 

Please post that patch now as well, just make a series out of this
socket compat_ioctl patch and your x25 patch so it becomes clear
that they depend on each other. It should be easier to justify the
infrastructure patch when there is an actual user for it ;-)

> @@ -143,6 +143,10 @@ struct proto_ops {
>                                       struct poll_table_struct *wait);
>         int             (*ioctl)     (struct socket *sock, unsigned int cmd,
>                                       unsigned long arg);
> +#ifdef CONFIG_COMPAT
> +       int             (*compat_ioctl) (struct socket *sock, unsigned int cmd,
> +                                     unsigned long arg);
> +#endif
>         int             (*listen)    (struct socket *sock, int len);
>         int             (*shutdown)  (struct socket *sock, int flags);
>         int             (*setsockopt)(struct socket *sock, int level,
...
> +
> +#define SOCKOPS_COMPAT_WRAP(name, fam)					\
> +SOCKCALL_WRAP(name, release, (struct socket *sock), (sock))	\
> +SOCKCALL_WRAP(name, bind, (struct socket *sock, struct sockaddr *uaddr,
> int addr_len), \
> +	      (sock, uaddr, addr_len))		

I really don't like the way you are extending the SOCKCALL_WRAP
mechanism like this. Ideally, you should convert the x25 layer to
not need SOCKOPS_WRAP at all.
Besides x25, only four other users of this remain, all others have
gotten rid of it. If you have a really good reason to keep it, it's
probably easier to add the compat_ioctl method unconditionally so
you don't need two different version of SOCKOPS_WRAP.
Making it unconditional would also help with those protocols that
have only compatible ioctl handlers and could then also point
.compat_ioctl to their native ioctl method without needing #ifdef
around it.

	Arnd <><

Re: [PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[Arnaldo Carvalho de Melo]

On 1/9/06, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 09 January 2006 06:46, Shaun Pereira wrote:

> > Since we are interested in ioctl's from userspace I have not added the
> > .compat_ioctl function pointer to struct net_device. The assumption
> > being once the userspace data has reached the kernel via the socket api,
> > if the socket layer protocol knows how to handle the data, it will
> > prepare it for the device.
>
> I think we need to have it in the long run, but if you don't need it
> for x25, then it's not your call to implement net_device->compat_ioctl.
> I've been thinking about how to get it right before, but did not
> reach a proper conclusion, since dev_ioctl is called in so many places
> that would all need to be changed for this.

Nowadays dev_ioctl is only called from one funcion: sock_ioctl in
net/socket.c, this is after a recent changeset by hch.

- Arnaldo

Re: [PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[Shaun Pereira]

And the correct x.25 patch, (will build a [PATCH] if this is ok).
Tested with with xot to a Cisco box. 
Thanks once again for your help 
/Shaun

Index: linux-2.6.15/net/x25/af_x25.c
===================================================================
--- linux-2.6.15.orig/net/x25/af_x25.c
+++ linux-2.6.15/net/x25/af_x25.c
@@ -54,6 +54,7 @@
 #include <linux/notifier.h>
 #include <linux/init.h>
 #include <net/x25.h>
+#include <linux/compat.h>
 
 int sysctl_x25_restart_request_timeout = X25_DEFAULT_T20;
 int sysctl_x25_call_request_timeout    = X25_DEFAULT_T21;
@@ -68,6 +69,14 @@ static struct proto_ops x25_proto_ops;
 
 static struct x25_address null_x25_address = {"               "};
 
+#ifdef CONFIG_COMPAT
+struct compat_x25_subscrip_struct {
+    char device[200-sizeof(compat_ulong_t)];
+    compat_ulong_t global_facil_mask;
+    compat_uint_t extended;
+};
+#endif
+
 int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr,
 		  struct x25_address *calling_addr)
 {
@@ -1386,6 +1395,106 @@ static struct net_proto_family x25_famil
 	.owner	=	THIS_MODULE,
 };
 
+
+#ifdef CONFIG_COMPAT
+static int compat_x25_subscr_ioctl(unsigned int cmd,
+       	struct compat_x25_subscrip_struct __user *x25_subscr32)
+{
+	struct x25_subscrip_struct x25_subscr;
+	struct x25_neigh *nb;
+	struct net_device *dev;
+	int rc = -EINVAL;
+
+	if (cmd != SIOCX25GSUBSCRIP && cmd != SIOCX25SSUBSCRIP)
+   		goto out;
+
+	rc = -EFAULT;
+	if(copy_from_user(&x25_subscr, x25_subscr32, sizeof(*x25_subscr32)))
+		goto out;
+
+	rc = -EINVAL;
+	if ((dev = x25_dev_get(x25_subscr.device)) == NULL)
+		goto out;
+
+	if ((nb = x25_get_neigh(dev)) == NULL)
+		goto out_dev_put;
+
+	dev_put(dev);
+
+	if(cmd == SIOCX25GSUBSCRIP) {
+		x25_subscr.extended             = nb->extended;
+		x25_subscr.global_facil_mask     = nb->global_facil_mask;
+		rc = copy_to_user(x25_subscr32, &x25_subscr,
+	    		sizeof(*x25_subscr32)) ? -EFAULT : 0;
+	} else {
+		rc = -EINVAL;
+		if (!(x25_subscr.extended && x25_subscr.extended != 1)) {
+			 rc = 0;
+			 nb->extended            = x25_subscr.extended;
+			 nb->global_facil_mask   = x25_subscr.global_facil_mask;
+		}
+	}
+	x25_neigh_put(nb);
+out:
+	return rc;
+out_dev_put:
+	dev_put(dev);
+	goto out;
+}
+
+static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
unsigned long arg)
+{
+	void __user *argp = compat_ptr(arg);
+
+	int rc = -ENOIOCTLCMD;
+
+	switch(cmd) {
+		case TIOCOUTQ:
+		case TIOCINQ:
+		case SIOCGSTAMP:
+		case SIOCGIFADDR:
+		case SIOCSIFADDR:
+		case SIOCGIFDSTADDR:
+		case SIOCSIFDSTADDR:
+		case SIOCGIFBRDADDR:
+		case SIOCSIFBRDADDR:
+		case SIOCGIFNETMASK:
+		case SIOCSIFNETMASK:
+		case SIOCGIFMETRIC:
+		case SIOCSIFMETRIC:
+		case SIOCADDRT:
+		case SIOCDELRT:
+		case SIOCX25GSUBSCRIP:
+			rc = compat_x25_subscr_ioctl(cmd, argp);
+			break;
+		case SIOCX25SSUBSCRIP:
+			rc = -EPERM;
+			if (!capable(CAP_NET_ADMIN))
+				break;
+			rc = compat_x25_subscr_ioctl(cmd, argp);
+			break;
+		case SIOCX25GFACILITIES:
+		case SIOCX25SFACILITIES:
+		case SIOCX25GCALLUSERDATA:
+		case SIOCX25SCALLUSERDATA:
+		case SIOCX25GCAUSEDIAG:
+		case SIOCX25SCUDMATCHLEN:
+		case SIOCX25CALLACCPTAPPRV:
+			rc = x25_ioctl(sock, cmd, (unsigned long)argp);
+			break;
+		case SIOCX25SENDCALLACCPT:
+			rc = x25_ioctl(sock, cmd, (unsigned long)argp);
+			break;
+		default:
+			rc = -ENOIOCTLCMD;
+			break;
+	}
+
+	return rc;
+}
+
+#endif
+
 static struct proto_ops SOCKOPS_WRAPPED(x25_proto_ops) = {
 	.family =	AF_X25,
 	.owner =	THIS_MODULE,
@@ -1397,6 +1506,9 @@ static struct proto_ops SOCKOPS_WRAPPED(
 	.getname =	x25_getname,
 	.poll =		datagram_poll,
 	.ioctl =	x25_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = compat_x25_ioctl,
+#endif
 	.listen =	x25_listen,
 	.shutdown =	sock_no_shutdown,
 	.setsockopt =	x25_setsockopt,

Re: [PATCH] net: 32 bit (socket layer) ioctl emulation for 64 bit kernels

[Arnd Bergmann]

On Wednesday 11 January 2006 06:28, Shaun Pereira wrote:
> And the correct x.25 patch, (will build a [PATCH] if this is ok).
> Tested with with xot to a Cisco box. 

Much better now, but

> +       switch(cmd) {
> +               case TIOCOUTQ:
> +               case TIOCINQ:

Looking at how these are handled in x25_ioctl(),
these should be forwarded to x25_ioctl(), because they are
compatible. With your current code you incorrectly return -EINVAL.

> +               case SIOCGSTAMP:

This one actually needs a conversion handler. You could
add a generic compat_sock_get_timestamp() function to net/compat.c
for this.

> +               case SIOCGIFADDR:
> +               case SIOCSIFADDR:
> +               case SIOCGIFDSTADDR:
> +               case SIOCSIFDSTADDR:
> +               case SIOCGIFBRDADDR:
> +               case SIOCSIFBRDADDR:
> +               case SIOCGIFNETMASK:
> +               case SIOCSIFNETMASK:
> +               case SIOCGIFMETRIC:
> +               case SIOCSIFMETRIC:

These all return -EINVAL in x25_ioctl, just do the same here.

For any the cases above, you can also choose not to handle them
in compat_x25_ioctl at all and just return -ENOIOCTLCMD, so they
get forwarded to the conversion code in fs/compat_ioctl.c. 

> +               case SIOCADDRT:
> +               case SIOCDELRT:

These should call x25_route_ioctl() instead of falling through to
to compat_x25_subscr_ioctl(), right?

> +               case SIOCX25GFACILITIES:
> +               case SIOCX25SFACILITIES:
> +               case SIOCX25GCALLUSERDATA:
> +               case SIOCX25SCALLUSERDATA:
> +               case SIOCX25GCAUSEDIAG:
> +               case SIOCX25SCUDMATCHLEN:
> +               case SIOCX25CALLACCPTAPPRV:
> +                       rc = x25_ioctl(sock, cmd, (unsigned long)argp);
> +                       break;
> +               case SIOCX25SENDCALLACCPT:
> +                       rc = x25_ioctl(sock, cmd, (unsigned long)argp);
> +                       break;

I guess these can be combined to a single case list.

        Arnd <><