Hello, Arnd!
Sorry for such delay, was on vacancy. Here is a patch, introducing
compat_(get|set)sockopt handlers, as you proposed.
On Tuesday 21 February 2006 14:56, Arnd Bergmann wrote:
> On Tuesday 21 February 2006 10:04, Dmitry Mishin wrote:
> > On Monday 20 February 2006 18:55, Arnd Bergmann wrote:
> > > Is CONFIG_COMPAT the right conditional here? If the code is only used
> > > for architectures that have different aligments, it should not need be
> > > compiled in for the other architectures.
> >
> > So, I'll define ARCH_HAS_FUNNY_64_ALIGNMENT in x86_64 and ia64 code and
> > will check it, as Andi suggested.
>
> I think nowadays, unconditionally setting CONFIG_FUNNY_64_ALIGNMENT from
> arch/{ia64,x86_64}/Kconfig would be the preferred way to a #define in
> include/asm.
>
> > > > #define IPT_ALIGN(s) XT_ALIGN(s)
> > > > +
> > > > +#ifdef CONFIG_COMPAT
> > > > +#include <net/compat.h>
> > > > +
> > > > +struct compat_ipt_getinfo
> > > > +{
> > > > + char name[IPT_TABLE_MAXNAMELEN];
> > > > + compat_uint_t valid_hooks;
> > > > + compat_uint_t hook_entry[NF_IP_NUMHOOKS];
> > > > + compat_uint_t underflow[NF_IP_NUMHOOKS];
> > > > + compat_uint_t num_entries;
> > > > + compat_uint_t size;
> > > > +};
> > >
> > > This structure looks like it does not need any
> > > conversions. You should probably just use
> > > struct ipt_getinfo then.
> >
> > I just saw compat_uint_t use in net/compat.c and thought, that it is a
> > good style to use it. Does anybody know arch, where sizeof(compat_uint_t)
> > != 4?
>
> No, the compat layer already heavily depends on the fact that compat_uint_t
> is always the same as unsigned int.
>
> > > Dito
> >
> > Disagree, ipt_entry_match and ipt_entry_target contain pointers which
> > make their alignment equal 8 byte on 64bits architectures.
>
> Ah, I see.
>
> > > I would much rather have either an extra 'compat' argument to to
> > > sock_setsockopt and proto_ops->setsockopt than to spread the use
> > > of is_compat_task further.
> >
> > Another weak place in my code. is_compat_task() approach has one
> > advantage - it doesn't require a lot of current code modifications.
> >
> > > Is the FIXME above the only reason that the code needs to be changed?
> > > What is the reason that you did not just address this in the
> > > compat_sys_setsockopt implementation?
> >
> > Code above doesn't work. iptables with version >= 1.3 does alignment
> > checks as well as kernel does. So, we can't simply put entries with 8
> > bytes alignment to userspace or with 4 bytes alignment to kernel - we
> > need translate them entry by entry. So, I tried to do this the most
> > correct way - that userspace will hide its alignment from kernel and vice
> > versa, with not only SET_REPLACE, but also GET_INFO, GET_ENTRIES and
> > SET_COUNTERS translation. First implementation was exactly in
> > compat_sys_setsockopt, but David asked me to do this in netfilter code
> > itself.
>
> Ok, I see the point there. It's probably best to push down all the
> conversions from compat_sys_setsockopt down to the protocol specific parts,
> similar to what we do for the ioctl handlers.
>
> I'm thinking of something like
>
> int compat_sock_setsockopt(struct socket *sock, int level, int optname,
> char __user *optval, int optlen)
> {
> switch (optname) {
> case SO_ATTACH_FILTER:
> return do_set_attach_filter(fd, level, optname,
> optval, optlen);
> case SO_SNDTIMEO:
> return do_set_sock_timeout(fd, level, optname,
> optval, optlen);
> default:
> break;
> }
> return sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> asmlinkage long compat_sys_setsockopt(int fd, int level, int optname,
> char __user *optval, int optlen)
> {
> int err;
> struct socket *sock;
>
> if (optlen < 0)
> return -EINVAL;
>
> if ((sock = sockfd_lookup(fd, &err))!=NULL)
> {
> err = security_socket_setsockopt(sock,level,optname);
> if (err) {
> sockfd_put(sock);
> return err;
> }
>
> if (level == SOL_SOCKET)
> err = compat_sock_setsockopt(sock, level,
> optname, optval, optlen);
> else if (sock->ops->compat_setsockopt)
> err = sock->ops->compat_setsockopt(sock, level,
> optname, optval, optlen);
> else
> err = sock->ops->setsockopt(sock, level,
> optname, optval, optlen);
> sockfd_put(sock);
> }
> return err;
> }
>
> int tcp_setsockopt(struct sock *sk, int level, int optname, char __user
> *optval, int optlen) {
> int err = 0;
>
> err = ip_setsockopt(sk, level, optname, optval, optlen);
>
> #ifdef CONFIG_NETFILTER
> if (err = -ENOPROTOOPT) {
> lock_sock(sk);
> err = nf_setsockopt(sk, PF_INET, optname, optval, optlen);
> release_sock(sk);
> }
> #endif
> return err;
> }
>
> int compat_tcp_setsockopt(struct sock *sk, int level, int optname, char
> __user *optval, int optlen) {
> int err = 0;
>
> err = ip_setsockopt(sk, level, optname, optval, optlen);
>
> #ifdef CONFIG_NETFILTER
> if (err = -ENOPROTOOPT) {
> lock_sock(sk);
> err = compat_nf_setsockopt(sk, PF_INET, optname, optval, optlen);
> release_sock(sk);
> }
> #endif
> return err;
> }
>
> And the same for udp, raw, ipv6, decnet and each of those with getsockopt.
> It is a bigger change, but it puts all the handlers where they belong
> and it is more extensible to other sockopt handlers if we find more
> fsckup in some of them.
>
> Arnd <><
--
Thanks,
Dmitry.
--- ./include/linux/net.h.compat 2006-03-07 11:22:27.000000000 +0300
+++ ./include/linux/net.h 2006-03-07 11:20:07.000000000 +0300
@@ -149,6 +149,12 @@ struct proto_ops {
int optname, char __user *optval, int optlen);
int (*getsockopt)(struct socket *sock, int level,
int optname, char __user *optval, int __user *optlen);
+#ifdef CONFIG_COMPAT
+ int (*compat_setsockopt)(struct socket *sock, int level,
+ int optname, char __user *optval, int optlen);
+ int (*compat_getsockopt)(struct socket *sock, int level,
+ int optname, char __user *optval, int __user *optlen);
+#endif
int (*sendmsg) (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len);
int (*recvmsg) (struct kiocb *iocb, struct socket *sock,
--- ./include/linux/netfilter.h.compat 2006-03-06 12:06:34.000000000 +0300
+++ ./include/linux/netfilter.h 2006-03-07 15:00:14.000000000 +0300
@@ -2,6 +2,7 @@
#define __LINUX_NETFILTER_H
#ifdef __KERNEL__
+#include <linux/config.h>
#include <linux/init.h>
#include <linux/types.h>
#include <linux/skbuff.h>
@@ -80,10 +81,18 @@ struct nf_sockopt_ops
int set_optmin;
int set_optmax;
int (*set)(struct sock *sk, int optval, void __user *user, unsigned int len);
+#ifdef CONFIG_COMPAT
+ int (*compat_set)(struct sock *sk, int optval,
+ void __user *user, unsigned int len);
+#endif
int get_optmin;
int get_optmax;
int (*get)(struct sock *sk, int optval, void __user *user, int *len);
+#ifdef CONFIG_COMPAT
+ int (*compat_get)(struct sock *sk, int optval,
+ void __user *user, int *len);
+#endif
/* Number of users inside set() or get(). */
unsigned int use;
@@ -246,6 +255,13 @@ int nf_setsockopt(struct sock *sk, int p
int nf_getsockopt(struct sock *sk, int pf, int optval, char __user *opt,
int *len);
+#ifdef CONFIG_COMPAT
+int compat_nf_setsockopt(struct sock *sk, int pf, int optval,
+ char __user *opt, int len);
+int compat_nf_getsockopt(struct sock *sk, int pf, int optval,
+ char __user *opt, int *len);
+#endif
+
/* Packet queuing */
struct nf_queue_handler {
int (*outfn)(struct sk_buff *skb, struct nf_info *info,
--- ./include/net/inet_connection_sock.h.compat 2006-03-06 12:06:34.000000000 +0300
+++ ./include/net/inet_connection_sock.h 2006-03-07 15:46:20.000000000 +0300
@@ -15,6 +15,7 @@
#ifndef _INET_CONNECTION_SOCK_H
#define _INET_CONNECTION_SOCK_H
+#include <linux/config.h>
#include <linux/compiler.h>
#include <linux/string.h>
#include <linux/timer.h>
@@ -50,6 +51,14 @@ struct inet_connection_sock_af_ops {
char __user *optval, int optlen);
int (*getsockopt)(struct sock *sk, int level, int optname,
char __user *optval, int __user *optlen);
+#ifdef CONFIG_COMPAT
+ int (*compat_setsockopt)(struct sock *sk,
+ int level, int optname,
+ char __user *optval, int optlen);
+ int (*compat_getsockopt)(struct sock *sk,
+ int level, int optname,
+ char __user *optval, int __user *optlen);
+#endif
void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
int sockaddr_len;
};
--- ./include/net/ip.h.compat 2006-03-06 12:06:34.000000000 +0300
+++ ./include/net/ip.h 2006-03-07 14:38:54.000000000 +0300
@@ -356,6 +356,12 @@ extern void ip_cmsg_recv(struct msghdr *
extern int ip_cmsg_send(struct msghdr *msg, struct ipcm_cookie *ipc);
extern int ip_setsockopt(struct sock *sk, int level, int optname, char __user *optval, int optlen);
extern int ip_getsockopt(struct sock *sk, int level, int optname, char __user *optval, int __user *optlen);
+#ifdef CONFIG_COMPAT
+extern int compat_ip_setsockopt(struct sock *sk, int level,
+ int optname, char __user *optval, int optlen);
+extern int compat_ip_getsocko
...
|
OpenVZ Forum
Members
Search
Help
Register
Login
Home
