Hi Arnd

I have made the changes suggested, and attached it below. I think it
should be good now.  
Just a couple of questions if I may. 
If I understand correctly from your comments (thanks for that, they are
helpful)
copy_to_user acts like a memcopy for an 'array' of bytes and should not
be used to copy the timeval struct to userspace. 
Rather put_user / __put_user macros should be used which allows transfer
of single element values of the structure. 
Does that also mean that copy_to_user should not be used in ioctl
calls? 

I was wondering if this the compat_sock_get_timestamp function is
needed? If I were to remove the SIOCGSTAMP case from the
compat_x25_ioctl function, then a SIOCGSTAMP ioctl system call would
return -ENOIOCTLCMD which could  then be handled by do_siocgstamp
handler in the ioctl32_hash_table? (fs/compat_ioctl.c)
In which case I could remove this patch from the rest of the series.
 
/Shaun


Index: linux-2.6.15/include/net/compat.h
===================================================================

--- linux-2.6.15.orig/include/net/compat.h
+++ linux-2.6.15/include/net/compat.h

@@ -23,6 +23,8 @@ struct compat_cmsghdr {
 	compat_int_t	cmsg_type;
 };
 
+extern int compat_sock_get_timestamp(struct sock *, struct timeval

__user *);
+
 #else /* defined(CONFIG_COMPAT) */
 #define compat_msghdr	msghdr		/* to avoid compiler warnings */
 #endif /* defined(CONFIG_COMPAT) */
Index: linux-2.6.15/net/compat.c
===================================================================

--- linux-2.6.15.orig/net/compat.c
+++ linux-2.6.15/net/compat.c

@@ -503,6 +503,25 @@ static int do_get_sock_timeout(int fd, i
 	return err;
 }
 
+int compat_sock_get_timestamp(struct sock *sk, struct timeval __user

*userstamp)
+{
+	struct compat_timeval __user *ctv
+		= (struct compat_timeval __user*) userstamp;
+	int err = -ENOENT;
+	if(!sock_flag(sk, SOCK_TIMESTAMP))
+		sock_enable_timestamp(sk);
+	if(sk->sk_stamp.tv_sec == -1)
+		return err;
+	if(sk->sk_stamp.tv_sec == 0)
+		do_gettimeofday(&sk->sk_stamp);
+	err = -EFAULT;
+	if(access_ok(VERIFTY_WRITE, ctv, sizeof(*ctv))) {
+		err = __put_user(sk->sk_stamp.tv_sec, &ctv->tv_sec);
+		err != __put_user(sk->sk_stamp.tv_usec, &ctv->tv_usec);
+	}
+	return err;
+}
+
 asmlinkage long compat_sys_getsockopt(int fd, int level, int optname,
 				char __user *optval, int __user *optlen)
 {

@@ -602,3 +621,5 @@ asmlinkage long compat_sys_socketcall(in
 	}
 	return ret;
 }
+
+EXPORT_SYMBOL(compat_sock_get_timestamp);

On Fri, 2006-01-13 at 11:46 +0000, Arnd Bergmann wrote:

On Friday 13 January 2006 03:14, Shaun Pereira wrote:

quoted

Thank you for reviewing that bit of code.  
I had a look at compat_sys_gettimeofday and sys32_gettimeofday codes.
They seem to work in a similar way, casting a pointer to the structure
from user space to a compat_timeval type.

The part with the case is ok, except that you don't have to write

struct compat_timeval __user *ctv;
ctv = (struct compat_timeval __user*) userstamp;

Instead,

struct compat_timeval __user *ctv = userstamp;

is the more common way to write it. The result is the same, since
userstamp is a 'void __user *'.

quoted

But to make sure I have tested the routine by forcing the sk-

quoted

sk_stamp.tv_sec value to 0 in the x25_module ( for testing purposes

only, as it is initialised to -1). Now when
I make a 32 bit userspace SIOCGSTAMP ioctl to the 64 bit kernel I should
get the current time back in user space. This seems to work, the ioctl
returns the system time (just after TEST6:)

So I have left the patch as is for now. However if necessary to use
the element-by-element __put_user routine as in put_tv32, then I can
make the change, just let me know.

You need to to exactly that, yes. I'm not sure what exactly you have
tested, but the expected result of your code would be that you see
the sk_stamp.tv_sec value in the output, but not the tv_usec value.

On little-endian system like x86_64, that is not much of a difference
(less than a second) that you might miss in a test case, but on
big-endian, it would be fatal.

The layout of the structures on most systems is 

		64 bit LE	64 bit BE	32 bit

bytes 0-3	tv_sec low	tv_sec high	tv_sec low
bytes 4-7	tv_sec high	tv_sec low	tv_usec low
bytes 8-11	tv_usec low	tv_usec high
bytes 12-15	tv_usec high	tv_usec low

You code copies the first eight bytes of the 64 bit data structure
into the 32 bit data structure.

	Arnd <><