sys_getdents64 needs compat wrapper ?

sys_getdents64 needs compat wrapper ?

[Arun Sharma]

Running 32 bit ls on ia64 produces kernel mode unaligned faults with the attached backtrace on 2.6.6. I'm curious why this isn't a problem for other compat archs.

	-Arun

> 
> 0xa0000001000405e0 ia64_handle_unaligned
>         args (0x805f7cc, 0xe000000105aa7bc0)
>         kernel 0xa0000001000405e0 0xa000000100040d00
> 0xa00000010000e650 ia64_prepare_handle_unaligned+0x30
>         args (0x805f7cc, 0xe000000105aa7bc0)
>         kernel 0xa00000010000e620 0xa00000010000e680
> 0xa00000010000e080 ia64_leave_kernel
>         kernel 0xa00000010000e080 0xa00000010000e2e0
> 0xa000000100142480 filldir64+0xa0
>         args (0xe000000105aa7e20, 0xe0000001fa100008, 0x1, 0x0, 0x14eb01)
>         kernel 0xa0000001001423e0 0xa000000100142680
> 0xa000000100203400 ext3_readdir+0x660
>         args (0xe0000001fae79c80, 0xe000000105aa7e20, 
> 0xa0000001006bc720, 0xa000
> 0001001423e0, 0xe0000001fa100004)
>         kernel 0xa000000100202da0 0xa000000100203660
> 0xa000000100141f20 vfs_readdir+0x160
>         args (0xe0000001fae79c80, 0xa0000001006bc720, 
> 0xe000000105aa7e20, 0xa000
> 000100920000, 0xe0000001fd3968e0)
>         kernel 0xa000000100141dc0 0xa000000100141f60
> 0xa000000100142760 sys_getdents64+0xe0
>         args (0x3, 0x805f7cc, 0x4000, 0x805f7b0, 0x4015fd98)
>         kernel 0xa000000100142680 0xa000000100142820
> 
> 

Re: sys_getdents64 needs compat wrapper ?

[David S. Miller]

On Thu, 20 May 2004 11:32:23 -0700
Arun Sharma <arun.sharma@intel.com> wrote:

> Running 32 bit ls on ia64 produces kernel mode unaligned faults with the attached backtrace on 2.6.6. I'm curious why this isn't a problem for other compat archs.

Other compat archs choose not to log unaligned kernel accesses
since they are common in some networking configurations and/or
combinations of drivers.

This one is a fixable case though, have a patch to suggest?
:-)

=?iso-8859-1?Q?Re:_sys_getdents64_needs_compat_wrapper_??=

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

Arnd Bergmann <arnd@arndb.de> schrieb am 20.05.2004, 22:54:01:
> 
> The attachment has an older (but so far untested) patch of mine
> to consolidate the syscalls from fs/readdir.c. 

Ok, so here goes the attachment (I hope). Stupid webmail...

[-- Attachment #2: compat_readdir.diff --]
[-- Type: text/plain, Size: 6293 bytes --]

===== fs/compat.c 1.24 vs edited =====
--- 1.24/fs/compat.c	Wed May 19 09:12:47 2004
+++ edited/fs/compat.c	Thu May 20 22:39:43 2004
@@ -1632,3 +1632,246 @@
 	return sys_ni_syscall();
 }
 #endif
+
+#define NAME_OFFSET(de) ((int) ((de)->d_name - (char *) (de)))
+#define COMPAT_ROUND_UP(x) (((x)+sizeof(compat_long_t)-1) & ~(sizeof(compat_long_t)-1))
+
+struct compat_old_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_offset;
+	unsigned short	d_namlen;
+	char		d_name[1];
+};
+
+struct compat_readdir_callback {
+	struct compat_old_linux_dirent __user * dirent;
+	int result;
+};
+
+static int compat_fillonedir(void * __buf, const char * name, int namlen, loff_t offset,
+		      ino_t ino, unsigned int d_type)
+{
+	struct compat_readdir_callback * buf = __buf;
+	struct compat_old_linux_dirent __user * dirent;
+
+	if (buf->result)
+		return -EINVAL;
+	buf->result++;
+	dirent = buf->dirent;
+	if (!access_ok(VERIFY_WRITE, (unsigned long)dirent,
+			(unsigned long)(dirent->d_name + namlen + 1) -
+				(unsigned long)dirent))
+		goto efault;
+	if (	__put_user(ino, &dirent->d_ino) ||
+		__put_user(offset, &dirent->d_offset) ||
+		__put_user(namlen, &dirent->d_namlen) ||
+		__copy_to_user(dirent->d_name, name, namlen) ||
+		__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	return 0;
+efault:
+	buf->result = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_old_readdir(unsigned int fd,
+	struct compat_old_linux_dirent __user * dirent, unsigned int count)
+{
+	int error;
+	struct file * file;
+	struct compat_readdir_callback buf;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.result = 0;
+	buf.dirent = dirent;
+
+	error = vfs_readdir(file, compat_fillonedir, &buf);
+	if (error >= 0)
+		error = buf.result;
+
+	fput(file);
+out:
+	return error;
+}
+
+struct compat_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_off;
+	unsigned short	d_reclen;
+	char		d_name[1];
+};
+
+struct compat_getdents_callback {
+	struct compat_linux_dirent __user * current_dir;
+	struct compat_linux_dirent __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir(void * __buf, const char * name, int namlen, loff_t offset,
+		   ino_t ino, unsigned int d_type)
+{
+	struct compat_linux_dirent __user * dirent;
+	struct compat_getdents_callback * buf = (struct compat_getdents_callback *) __buf;
+	int reclen = COMPAT_ROUND_UP(NAME_OFFSET(dirent) + namlen + 2);
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+	if (dirent) {
+		if (__put_user(offset, &dirent->d_off))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if (__put_user(ino, &dirent->d_ino))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	if (__put_user(d_type, (char *) dirent + reclen - 1))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents(unsigned int fd, struct compat_linux_dirent __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct compat_linux_dirent __user * lastdirent;
+	struct compat_getdents_callback buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		if (put_user(file->f_pos, &lastdirent->d_off))
+			error = -EFAULT;
+		else
+			error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+
+#ifndef __ARCH_OMIT_COMPAT_SYS_GETDENTS64
+#define COMPAT_ROUND_UP64(x) (((x)+sizeof(u64)-1) & ~(sizeof(u64)-1))
+
+struct compat_getdents_callback64 {
+	struct linux_dirent64 __user * current_dir;
+	struct linux_dirent64 __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir64(void * __buf, const char * name, int namlen, loff_t offset,
+		     ino_t ino, unsigned int d_type)
+{
+	struct linux_dirent64 __user *dirent;
+	struct compat_getdents_callback64 * buf = (struct compat_getdents_callback64 *) __buf;
+	int reclen = COMPAT_ROUND_UP64(NAME_OFFSET(dirent) + namlen + 1);
+	u64 off;
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+	if (dirent) {
+		if (__put_user(offset, &dirent->d_off))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if (__copy_to_user(&dirent->d_ino, &ino, sizeof(ino)))
+		goto efault;
+	off = 0;
+	if (__copy_to_user(&dirent->d_off, &off, sizeof(off)))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (__put_user(d_type, &dirent->d_type))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents64(unsigned int fd, struct linux_dirent64 __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct linux_dirent64 __user * lastdirent;
+	struct compat_getdents_callback64 buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir64, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		typeof(lastdirent->d_off) d_off = file->f_pos;
+		__put_user(d_off, &lastdirent->d_off);
+		error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+#endif /* ! __ARCH_OMIT_COMPAT_SYS_GETDENTS64 */

Re: =?iso-8859-1?Q?Re:_sys_getdents64_needs_compat_wrapper_??=

[Arun Sharma]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On 5/20/2004 2:06 PM, Arnd Bergmann wrote:
> Arnd Bergmann <arnd@arndb.de> schrieb am 20.05.2004, 22:54:01:
> 
>>The attachment has an older (but so far untested) patch of mine
>>to consolidate the syscalls from fs/readdir.c. 
> 

Thanks for the patch. We made a few changes to the patch:

- use put_user instead of copy_to_user for simple data types.
- Couple of more instances where a 64 bit copy needs to be split into two

compat_filldir64():	__put_user(offset, &dirent->d_off)
compat_sys_getdents64():	__put_user(d_off, &lastdirent->d_off)

With these changes, we don't see unaligned faults anymore.

	-Arun


[-- Attachment #2: readdir_ive.1.diff --]
[-- Type: text/plain, Size: 7576 bytes --]

diff -Nurap a/arch/ia64/ia32/ia32_entry.S b/arch/ia64/ia32/ia32_entry.S
--- a/arch/ia64/ia32/ia32_entry.S	2004-05-10 10:32:38.000000000 +0800
+++ b/arch/ia64/ia32/ia32_entry.S	2004-06-01 16:08:09.268146780 +0800
@@ -349,7 +349,7 @@ ia32_syscall_table:
 	data8 sys_setfsuid	/* 16-bit version */
 	data8 sys_setfsgid	/* 16-bit version */
 	data8 sys_llseek	  /* 140 */
-	data8 sys32_getdents
+	data8 compat_sys_getdents
 	data8 sys32_select
 	data8 sys_flock
 	data8 sys32_msync
@@ -428,7 +428,7 @@ ia32_syscall_table:
 	data8 sys_pivot_root
 	data8 sys_mincore
 	data8 sys_madvise
-	data8 sys_getdents64	  /* 220 */
+	data8 compat_sys_getdents64	  /* 220 */
 	data8 compat_sys_fcntl64
 	data8 sys_ni_syscall		/* reserved for TUX */
 	data8 sys_ni_syscall		/* reserved for Security */
diff -Nurap a/fs/compat.c b/fs/compat.c
--- a/fs/compat.c	2004-05-10 10:32:29.000000000 +0800
+++ b/fs/compat.c	2004-06-01 16:08:38.474201110 +0800
@@ -34,6 +34,7 @@
 #include <linux/syscalls.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
+#include <linux/dirent.h>
 #include <net/sock.h>		/* siocdevprivate_ioctl */
 
 #include <asm/uaccess.h>
@@ -794,3 +795,252 @@ asmlinkage int compat_sys_mount(char __u
 	return retval;
 }
 
+#define NAME_OFFSET(de) ((int) ((de)->d_name - (char *) (de)))
+#define COMPAT_ROUND_UP(x) (((x)+sizeof(compat_long_t)-1) & ~(sizeof(compat_long_t)-1))
+
+struct compat_old_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_offset;
+	unsigned short	d_namlen;
+	char		d_name[1];
+};
+
+struct compat_readdir_callback {
+	struct compat_old_linux_dirent __user * dirent;
+	int result;
+};
+
+static int compat_fillonedir(void * __buf, const char * name, int namlen, loff_t offset,
+		      ino_t ino, unsigned int d_type)
+{
+	struct compat_readdir_callback * buf = __buf;
+	struct compat_old_linux_dirent __user * dirent;
+
+	if (buf->result)
+		return -EINVAL;
+	buf->result++;
+	dirent = buf->dirent;
+	if (!access_ok(VERIFY_WRITE, (unsigned long)dirent,
+			(unsigned long)(dirent->d_name + namlen + 1) -
+				(unsigned long)dirent))
+		goto efault;
+	if (	__put_user(ino, &dirent->d_ino) ||
+		__put_user(offset, &dirent->d_offset) ||
+		__put_user(namlen, &dirent->d_namlen) ||
+		__copy_to_user(dirent->d_name, name, namlen) ||
+		__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	return 0;
+efault:
+	buf->result = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_old_readdir(unsigned int fd,
+	struct compat_old_linux_dirent __user * dirent, unsigned int count)
+{
+	int error;
+	struct file * file;
+	struct compat_readdir_callback buf;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.result = 0;
+	buf.dirent = dirent;
+
+	error = vfs_readdir(file, compat_fillonedir, &buf);
+	if (error >= 0)
+		error = buf.result;
+
+	fput(file);
+out:
+	return error;
+}
+
+struct compat_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_off;
+	unsigned short	d_reclen;
+	char		d_name[1];
+};
+
+struct compat_getdents_callback {
+	struct compat_linux_dirent __user * current_dir;
+	struct compat_linux_dirent __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir(void * __buf, const char * name, int namlen, loff_t offset,
+		   ino_t ino, unsigned int d_type)
+{
+	struct compat_linux_dirent __user * dirent;
+	struct compat_getdents_callback * buf = (struct compat_getdents_callback *) __buf;
+	int reclen = COMPAT_ROUND_UP(NAME_OFFSET(dirent) + namlen + 2);
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+	if (dirent) {
+		if (__put_user(offset, &dirent->d_off))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if (__put_user(ino, &dirent->d_ino))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	if (__put_user(d_type, (char *) dirent + reclen - 1))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents(unsigned int fd, struct compat_linux_dirent __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct compat_linux_dirent __user * lastdirent;
+	struct compat_getdents_callback buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		if (put_user(file->f_pos, &lastdirent->d_off))
+			error = -EFAULT;
+		else
+			error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+
+#ifndef __ARCH_OMIT_COMPAT_SYS_GETDENTS64
+#define COMPAT_ROUND_UP64(x) (((x)+sizeof(u64)-1) & ~(sizeof(u64)-1))
+
+struct compat_getdents_callback64 {
+	struct linux_dirent64 __user * current_dir;
+	struct linux_dirent64 __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir64(void * __buf, const char * name, int namlen, loff_t offset,
+		     ino_t ino, unsigned int d_type)
+{
+	struct linux_dirent64 __user *dirent;
+	struct compat_getdents_callback64 * buf = (struct compat_getdents_callback64 *) __buf;
+	int jj = NAME_OFFSET(dirent);
+	int reclen = COMPAT_ROUND_UP64(jj + namlen + 1);
+	u64 off;
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+
+	if (dirent) {
+		if ((__put_user(offset, (u32 *)&dirent->d_off))
+		 || (__put_user(offset >> 32, ((u32 *)&dirent->d_off) + 1)))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if ((__put_user(ino, (u32 *)&dirent->d_ino))
+	 || (__put_user(ino >> 32, ((u32 *)&dirent->d_ino) + 1)))
+		goto efault;
+	off = 0;
+	if ((__put_user(off, (u32 *)&dirent->d_off))
+	 || (__put_user(off >> 32, ((u32 *)&dirent->d_off) + 1)))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (__put_user(d_type, &dirent->d_type))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents64(unsigned int fd, struct linux_dirent64 __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct linux_dirent64 __user * lastdirent;
+	struct compat_getdents_callback64 buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir64, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		typeof(lastdirent->d_off) d_off = file->f_pos;
+		__put_user(d_off, (u32 *)&lastdirent->d_off);
+		__put_user(d_off >> 32, ((u32 *)&lastdirent->d_off) + 1);
+		error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+#endif /* ! __ARCH_OMIT_COMPAT_SYS_GETDENTS64 */
+

Re: sys getdents64 needs compat wrapper ?

[David S. Miller]

Some bug fixes needed:

+#define NAME_OFFSET(de) ((int) ((de)->d_name - (char *) (de)))

Cast must be "(char __user *)" to avoid sparse warnings.

+	if (__put_user(d_type, (char *) dirent + reclen - 1))

Again need "(char __user *)" for sparse.

+		if ((__put_user(offset, (u32 *)&dirent->d_off))
+		 || (__put_user(offset >> 32, ((u32 *)&dirent->d_off) + 1)))

Again, need "(u32 __user *)"

	 || (__put_user(ino >> 32, ((u32 *)&dirent->d_ino) + 1)))

Another "(u32 __user *)"

+	 || (__put_user(off >> 32, ((u32 *)&dirent->d_off) + 1)))

Another "(u32 __user *)"

+		__put_user(d_off, (u32 *)&lastdirent->d_off);
+		__put_user(d_off >> 32, ((u32 *)&lastdirent->d_off) + 1);

Two more "(u32 __user *)"

People really need to get into the habit of running sparse on their
changes to the compat code now that we've annotated most of it. :-)

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On 6/4/2004 5:28 PM, David S. Miller wrote:

> Some bug fixes needed:
> 
> +#define NAME_OFFSET(de) ((int) ((de)->d_name - (char *) (de)))
> 
> Cast must be "(char __user *)" to avoid sparse warnings.

This updated patch takes care of the __user problems that Dave pointed out. Regarding __put_user vs __copy_to_user, __put_user is more optimal on ia64 like Andrew already noted. I realize it's not optimal on all platforms. So I suggest that we apply the patch for now and try to build something that's optimal for all platforms in the future.

	-Arun


[-- Attachment #2: readdir_ive.3.patch --]
[-- Type: text/plain, Size: 7933 bytes --]

Running /emul/ia32-linux/bin/ls on 2.6.6 produces kernel mode unaligned
faults.  Analysis shows it's caused by getdents64 system call.

Signed-off-by: Arun Sharma <arun.sharma@intel.com>
Signed-off-by: Gordon Jin <gordon.jin@intel.com>
Original patch by: Arnd Bergmann <arnd@arndb.de>

diff -Nurap a/arch/ia64/ia32/ia32_entry.S b/arch/ia64/ia32/ia32_entry.S
--- a/arch/ia64/ia32/ia32_entry.S	2004-05-10 10:32:38.000000000 +0800
+++ b/arch/ia64/ia32/ia32_entry.S	2004-06-07 17:42:56.151928752 +0800
@@ -349,7 +349,7 @@ ia32_syscall_table:
 	data8 sys_setfsuid	/* 16-bit version */
 	data8 sys_setfsgid	/* 16-bit version */
 	data8 sys_llseek	  /* 140 */
-	data8 sys32_getdents
+	data8 compat_sys_getdents
 	data8 sys32_select
 	data8 sys_flock
 	data8 sys32_msync
@@ -428,7 +428,7 @@ ia32_syscall_table:
 	data8 sys_pivot_root
 	data8 sys_mincore
 	data8 sys_madvise
-	data8 sys_getdents64	  /* 220 */
+	data8 compat_sys_getdents64	  /* 220 */
 	data8 compat_sys_fcntl64
 	data8 sys_ni_syscall		/* reserved for TUX */
 	data8 sys_ni_syscall		/* reserved for Security */
diff -Nurap a/fs/compat.c b/fs/compat.c
--- a/fs/compat.c	2004-05-10 10:32:29.000000000 +0800
+++ b/fs/compat.c	2004-06-07 17:43:23.478100292 +0800
@@ -34,6 +34,7 @@
 #include <linux/syscalls.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
+#include <linux/dirent.h>
 #include <net/sock.h>		/* siocdevprivate_ioctl */
 
 #include <asm/uaccess.h>
@@ -794,3 +795,252 @@ asmlinkage int compat_sys_mount(char __u
 	return retval;
 }
 
+#define NAME_OFFSET(de) ((int) ((de)->d_name - (char __user *) (de)))
+#define COMPAT_ROUND_UP(x) (((x)+sizeof(compat_long_t)-1) & ~(sizeof(compat_long_t)-1))
+
+struct compat_old_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_offset;
+	unsigned short	d_namlen;
+	char		d_name[1];
+};
+
+struct compat_readdir_callback {
+	struct compat_old_linux_dirent __user * dirent;
+	int result;
+};
+
+static int compat_fillonedir(void * __buf, const char * name, int namlen, loff_t offset,
+		      ino_t ino, unsigned int d_type)
+{
+	struct compat_readdir_callback * buf = __buf;
+	struct compat_old_linux_dirent __user * dirent;
+
+	if (buf->result)
+		return -EINVAL;
+	buf->result++;
+	dirent = buf->dirent;
+	if (!access_ok(VERIFY_WRITE, (unsigned long)dirent,
+			(unsigned long)(dirent->d_name + namlen + 1) -
+				(unsigned long)dirent))
+		goto efault;
+	if (	__put_user(ino, &dirent->d_ino) ||
+		__put_user(offset, &dirent->d_offset) ||
+		__put_user(namlen, &dirent->d_namlen) ||
+		__copy_to_user(dirent->d_name, name, namlen) ||
+		__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	return 0;
+efault:
+	buf->result = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_old_readdir(unsigned int fd,
+	struct compat_old_linux_dirent __user * dirent, unsigned int count)
+{
+	int error;
+	struct file * file;
+	struct compat_readdir_callback buf;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.result = 0;
+	buf.dirent = dirent;
+
+	error = vfs_readdir(file, compat_fillonedir, &buf);
+	if (error >= 0)
+		error = buf.result;
+
+	fput(file);
+out:
+	return error;
+}
+
+struct compat_linux_dirent {
+	compat_ulong_t	d_ino;
+	compat_ulong_t	d_off;
+	unsigned short	d_reclen;
+	char		d_name[1];
+};
+
+struct compat_getdents_callback {
+	struct compat_linux_dirent __user * current_dir;
+	struct compat_linux_dirent __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir(void * __buf, const char * name, int namlen, loff_t offset,
+		   ino_t ino, unsigned int d_type)
+{
+	struct compat_linux_dirent __user * dirent;
+	struct compat_getdents_callback * buf = (struct compat_getdents_callback *) __buf;
+	int reclen = COMPAT_ROUND_UP(NAME_OFFSET(dirent) + namlen + 2);
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+	if (dirent) {
+		if (__put_user(offset, &dirent->d_off))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if (__put_user(ino, &dirent->d_ino))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	if (__put_user(d_type, (char  __user *) dirent + reclen - 1))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents(unsigned int fd, struct compat_linux_dirent __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct compat_linux_dirent __user * lastdirent;
+	struct compat_getdents_callback buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		if (put_user(file->f_pos, &lastdirent->d_off))
+			error = -EFAULT;
+		else
+			error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+
+#ifndef __ARCH_OMIT_COMPAT_SYS_GETDENTS64
+#define COMPAT_ROUND_UP64(x) (((x)+sizeof(u64)-1) & ~(sizeof(u64)-1))
+
+struct compat_getdents_callback64 {
+	struct linux_dirent64 __user * current_dir;
+	struct linux_dirent64 __user * previous;
+	int count;
+	int error;
+};
+
+static int compat_filldir64(void * __buf, const char * name, int namlen, loff_t offset,
+		     ino_t ino, unsigned int d_type)
+{
+	struct linux_dirent64 __user *dirent;
+	struct compat_getdents_callback64 * buf = (struct compat_getdents_callback64 *) __buf;
+	int jj = NAME_OFFSET(dirent);
+	int reclen = COMPAT_ROUND_UP64(jj + namlen + 1);
+	u64 off;
+
+	buf->error = -EINVAL;	/* only used if we fail.. */
+	if (reclen > buf->count)
+		return -EINVAL;
+	dirent = buf->previous;
+
+	if (dirent) {
+		if ((__put_user(offset, (u32 __user *)&dirent->d_off))
+		 || (__put_user(offset >> 32, ((u32 __user *)&dirent->d_off) + 1)))
+			goto efault;
+	}
+	dirent = buf->current_dir;
+	if ((__put_user(ino, (u32 __user *)&dirent->d_ino))
+	 || (__put_user(ino >> 32, ((u32 __user *)&dirent->d_ino) + 1)))
+		goto efault;
+	off = 0;
+	if ((__put_user(off, (u32 __user *)&dirent->d_off))
+	 || (__put_user(off >> 32, ((u32 __user *)&dirent->d_off) + 1)))
+		goto efault;
+	if (__put_user(reclen, &dirent->d_reclen))
+		goto efault;
+	if (__put_user(d_type, &dirent->d_type))
+		goto efault;
+	if (copy_to_user(dirent->d_name, name, namlen))
+		goto efault;
+	if (__put_user(0, dirent->d_name + namlen))
+		goto efault;
+	buf->previous = dirent;
+	dirent = (void __user *)dirent + reclen;
+	buf->current_dir = dirent;
+	buf->count -= reclen;
+	return 0;
+efault:
+	buf->error = -EFAULT;
+	return -EFAULT;
+}
+
+asmlinkage long compat_sys_getdents64(unsigned int fd, struct linux_dirent64 __user * dirent, unsigned int count)
+{
+	struct file * file;
+	struct linux_dirent64 __user * lastdirent;
+	struct compat_getdents_callback64 buf;
+	int error;
+
+	error = -EFAULT;
+	if (!access_ok(VERIFY_WRITE, dirent, count))
+		goto out;
+
+	error = -EBADF;
+	file = fget(fd);
+	if (!file)
+		goto out;
+
+	buf.current_dir = dirent;
+	buf.previous = NULL;
+	buf.count = count;
+	buf.error = 0;
+
+	error = vfs_readdir(file, compat_filldir64, &buf);
+	if (error < 0)
+		goto out_putf;
+	error = buf.error;
+	lastdirent = buf.previous;
+	if (lastdirent) {
+		typeof(lastdirent->d_off) d_off = file->f_pos;
+		__put_user(d_off, (u32 __user *)&lastdirent->d_off);
+		__put_user(d_off >> 32, ((u32 __user *)&lastdirent->d_off) + 1);
+		error = count - buf.count;
+	}
+
+out_putf:
+	fput(file);
+out:
+	return error;
+}
+#endif /* ! __ARCH_OMIT_COMPAT_SYS_GETDENTS64 */
+

Re: sys getdents64 needs compat wrapper ?

[David S. Miller]

On Mon, 07 Jun 2004 14:13:02 -0700
Arun Sharma <arun.sharma@intel.com> wrote:

> This updated patch takes care of the __user problems that Dave pointed
> out. Regarding __put_user vs __copy_to_user, __put_user is more
> optimal on ia64 like Andrew already noted. I realize it's not optimal
> on all platforms. So I suggest that we apply the patch for now and try
> to build something that's optimal for all platforms in the future.

I agree.

Re: sys getdents64 needs compat wrapper ?

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

On maanantai 07 kesäkuu 2004 23:13, Arun Sharma wrote:
> This updated patch takes care of the __user problems that Dave
> pointed out. Regarding __put_user vs __copy_to_user, __put_user
> is more optimal on ia64 like Andrew already noted. I realize it's
> not optimal on all platforms. So I suggest that we apply the 
> patch for now and try to build something that's optimal for all
> platforms in the future.     

> +       if (dirent) {
> +               if ((__put_user(offset, (u32 __user *)&dirent->d_off))
> +                || (__put_user(offset >> 32, ((u32 __user *)&dirent->d_off) + 1)))
> +                       goto efault;
> +       }

I just realized your code is broken on big-endian architectures, so
we should either get back to __copy_to_user or find the optimal solution
now.

How about a {__,}{get,put}_user_unaligned() that gets defined per
architecture in one of these two ways:

/* put_user doesn't care about alignment */
#define put_user_unaligned(x,ptr) put_user(x,ptr)

/* put_user only works on aligned data */
#define put_user_unaligned(x,ptr) ({ \
	__typeof__(*(ptr)) __x = (x); \
	copy_to_user(ptr, &__x, sizeof(*(ptr))) ? -EFAULT : 0; \
})

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

On 6/11/2004 8:09 AM, Arnd Bergmann wrote:
> 
>>+       if (dirent) {
>>+               if ((__put_user(offset, (u32 __user *)&dirent->d_off))
>>+                || (__put_user(offset >> 32, ((u32 __user *)&dirent->d_off) + 1)))
>>+                       goto efault;
>>+       }
> 
> 
> I just realized your code is broken on big-endian architectures, so
> we should either get back to __copy_to_user or find the optimal solution
> now.
> 
> How about a {__,}{get,put}_user_unaligned() that gets defined per
> architecture in one of these two ways:

We'll go for the optimal solution now. We'll provide an <asm-generic/uaccess.h> that does __copy_to_user and a <asm-ia64/uaccess.h> that's optimized for ia64. Archs which don't care about alignment can override appropriately.

Should have the patch tested by tomorrow.

	-Arun

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 6/14/2004 11:15 AM, Arun Sharma wrote:

>> I just realized your code is broken on big-endian architectures, so
>> we should either get back to __copy_to_user or find the optimal solution
>> now.
>>
>> How about a {__,}{get,put}_user_unaligned() that gets defined per
>> architecture in one of these two ways:
> 
> 
> We'll go for the optimal solution now. We'll provide an 
> <asm-generic/uaccess.h> that does __copy_to_user and a 
> <asm-ia64/uaccess.h> that's optimized for ia64. Archs which don't care 
> about alignment can override appropriately.
> 
> Should have the patch tested by tomorrow.

It took us a bit longer :) But here's the promised patch. Using __put_user_unaligned() on ia64 may still cause unaligned faults, but we chose to optimize for the common case, where it's 4 byte aligned.

	-Arun

[-- Attachment #2: __get_put_user_unaligned.patch --]
[-- Type: text/plain, Size: 8037 bytes --]

(1) Fix compat_filldir64() which is broken on big-endian machines.
(2) Introduce new APIs __{get,put}_user_unaligned.
(3) Optimize __{get,put}_user_unaligned for ia64.

Signed-off-by: Gordon Jin <gordon.jin@intel.com>
Signed-off-by: Arun Sharma <arun.sharma@intel.com>

---

 fs/compat.c                   |   14 ++++----------
 include/asm-generic/uaccess.h |   25 +++++++++++++++++++++++++
 include/asm-ia64/uaccess.h    |   36 ++++++++++++++++++++++++++++++++++++
 include/asm-mips/uaccess.h    |    1 +
 include/asm-ppc64/uaccess.h   |    1 +
 include/asm-s390/uaccess.h    |    1 +
 include/asm-sparc64/uaccess.h |    1 +
 include/asm-x86_64/uaccess.h  |    1 +
 8 files changed, 70 insertions(+), 10 deletions(-)

diff -purN -X dontdiff linux-2.6.7-rc3-getdents/fs/compat.c linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c
--- linux-2.6.7-rc3-getdents/fs/compat.c	2004-06-16 22:13:46.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c	2004-06-17 14:51:42.019623885 +0800
@@ -979,19 +979,14 @@ static int compat_filldir64(void * __buf
 	dirent = buf->previous;
 
 	if (dirent) {
-		if (__put_user(offset, (u32 __user *)&dirent->d_off))
-			goto efault;
-		if (__put_user(offset >> 32,
-				((u32 __user *)&dirent->d_off) + 1))
+		if (__put_user_unaligned(offset, &dirent->d_off))
 			goto efault;
 	}
 	dirent = buf->current_dir;
-	if ((__put_user(ino, (u32 __user *)&dirent->d_ino))
-	 || (__put_user(ino >> 32, ((u32 __user *)&dirent->d_ino) + 1)))
+	if (__put_user_unaligned(ino, &dirent->d_ino))
 		goto efault;
 	off = 0;
-	if ((__put_user(off, (u32 __user *)&dirent->d_off))
-	 || (__put_user(off >> 32, ((u32 __user *)&dirent->d_off) + 1)))
+	if (__put_user_unaligned(off, &dirent->d_off))
 		goto efault;
 	if (__put_user(reclen, &dirent->d_reclen))
 		goto efault;
@@ -1040,8 +1035,7 @@ asmlinkage long compat_sys_getdents64(un
 	lastdirent = buf.previous;
 	if (lastdirent) {
 		typeof(lastdirent->d_off) d_off = file->f_pos;
-		__put_user(d_off, (u32 __user *)&lastdirent->d_off);
-		__put_user(d_off >> 32, ((u32 __user *)&lastdirent->d_off) + 1);
+		__put_user_unaligned(d_off, &lastdirent->d_off);
 		error = count - buf.count;
 	}
 
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h	2004-06-17 15:06:28.391683339 +0800
@@ -0,0 +1,25 @@
+#ifndef _ASM_GENERIC_UACCESS_H_
+#define _ASM_GENERIC_UACCESS_H_
+
+/* 
+ * This macro should be used instead of __get_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __get_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x = (x);					\
+	copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;	\
+})
+
+
+/* 
+ * This macro should be used instead of __put_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __put_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x = (x);					\
+	copy_to_user((ptr), &__x, sizeof(*(ptr))) ? -EFAULT : 0;	\
+})
+
+#endif /* _ASM_GENERIC_UACCESS_H */
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h	2004-06-16 22:13:46.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h	2004-06-17 15:13:34.646560930 +0800
@@ -91,6 +91,42 @@ verify_area (int type, const void *addr,
 #define __put_user(x, ptr)	__put_user_nocheck((__typeof__(*(ptr))) (x), (ptr), sizeof(*(ptr)))
 #define __get_user(x, ptr)	__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
 
+extern long __put_user_unaligned_unknown (void);
+
+#define __put_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __put_user((x), (ptr)); break;					\
+		case 2: __ret = (__put_user((x), (u8 __user *)(ptr)))				\
+			|| (__put_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__put_user((x), (u16 __user *)(ptr)))				\
+			|| (__put_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__put_user((x), (u32 __user *)(ptr)))				\
+			|| (__put_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __put_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
+extern long __get_user_unaligned_unknown (void);
+
+#define __get_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __get_user((x), (ptr)); break;					\
+		case 2: __ret = (__get_user((x), (u8 __user *)(ptr)))				\
+			|| (__get_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__get_user((x), (u16 __user *)(ptr)))				\
+			|| (__get_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__get_user((x), (u32 __user *)(ptr)))				\
+			|| (__get_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __get_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
 #ifdef ASM_SUPPORTED
   struct __large_struct { unsigned long buf[100]; };
 # define __m(x) (*(struct __large_struct *)(x))
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h	2004-06-15 13:41:35.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h	2004-06-17 14:51:42.020600447 +0800
@@ -13,6 +13,7 @@
 #include <linux/compiler.h>
 #include <linux/errno.h>
 #include <linux/thread_info.h>
+#include <asm-generic/uaccess.h>
 
 /*
  * The fs value determines whether argument validity checking should be
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h	2004-06-15 13:41:35.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h	2004-06-17 14:51:42.020600447 +0800
@@ -12,6 +12,7 @@
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <asm/processor.h>
+#include <asm-generic/uaccess.h>
 
 #define VERIFY_READ	0
 #define VERIFY_WRITE	1
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h	2004-06-15 13:41:39.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h	2004-06-17 14:51:42.021577010 +0800
@@ -16,6 +16,7 @@
  */
 #include <linux/sched.h>
 #include <linux/errno.h>
+#include <asm-generic/uaccess.h>
 
 #define VERIFY_READ     0
 #define VERIFY_WRITE    1
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h	2004-06-15 13:41:39.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h	2004-06-17 14:51:42.021577010 +0800
@@ -14,6 +14,7 @@
 #include <asm/asi.h>
 #include <asm/system.h>
 #include <asm/spitfire.h>
+#include <asm-generic/uaccess.h>
 #endif
 
 #ifndef __ASSEMBLY__
diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h	2004-06-15 13:41:34.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h	2004-06-17 14:51:42.022553572 +0800
@@ -10,6 +10,7 @@
 #include <linux/sched.h>
 #include <linux/prefetch.h>
 #include <asm/page.h>
+#include <asm-generic/uaccess.h>
 
 #define VERIFY_READ 0
 #define VERIFY_WRITE 1

Re: sys getdents64 needs compat wrapper ?

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 5306 bytes --]

On Freitag, 18. Juni 2004 00:28, Arun Sharma wrote:
> It took us a bit longer :) But here's the promised patch. Using
> __put_user_unaligned() on ia64 may still cause unaligned faults,
> but we chose to optimize for the common case, where it's 4 byte
> aligned.   

Isn't it legal for i386 code to pass syscall arguments with
an arbitrary alignment? I guess gcc normally aligns everything
to 32 bit unless you force it not to but you might still return
error for syscalls that work fine on a native i386 system.

> + */
> +#define __get_user_unaligned(x, ptr)                                   \
> +({                                                                     \
> +       __typeof__ (*(ptr)) __x = (x);                                  \
> +       copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;      \
> +})

This should be __copy_from_user instead of copy_from_user, right?

> +
> +
> +/* 
> + * This macro should be used instead of __put_user() when accessing
> + * values at locations that are unknown to be aligned.
> + */
> +#define __put_user_unaligned(x, ptr)                                   \
> +({                                                                     \
> +       __typeof__ (*(ptr)) __x = (x);                                  \
> +       copy_to_user((ptr), &__x, sizeof(*(ptr))) ? -EFAULT : 0;        \
> +})

same here.

> +extern long __put_user_unaligned_unknown (void);
> +
> +#define __put_user_unaligned(x, ptr)                                                           \
> +({                                                                                             \
> +       long __ret;                                                                             \
> +       switch (sizeof(*(ptr))) {                                                               \
> +               case 1: __ret = __put_user((x), (ptr)); break;                                  \
> +               case 2: __ret = (__put_user((x), (u8 __user *)(ptr)))                           \
> +                       || (__put_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;             \
> +               case 4: __ret = (__put_user((x), (u16 __user *)(ptr)))                          \
> +                       || (__put_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;           \
> +               case 8: __ret = (__put_user((x), (u32 __user *)(ptr)))                          \
> +                       || (__put_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;           \
> +               default: __ret = __put_user_unaligned_unknown();                                \
> +       }                                                                                       \
> +       __ret;                                                                                  \
> +})

Under what circumstances would you need to break down a 4 byte
alignment into 2 bytes? I can understand that you want to optimize
the unaligned-64 bit case because i386-gcc aligns them only to 32 bit
in user space, but the other special cases are bogus. Why not
just do:

#define __put_user_unaligned(x, ptr)					\
(									\
	(sizeof(*(ptr)) == 8) ?						\
		((__put_user((x), (u32 __user *)(ptr)))			\
		 | (__put_user((x) >> 32, ((u32 __user *)(ptr) + 1))))	\
		: __put_user(x,ptr)					\
)

Note also the use of | instead of || to get the correct value on error
(-EFAULT, not 1).

> diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h
> --- linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h        2004-06-15 13:41:35.000000000 +0800
> +++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h 2004-06-17 14:51:42.020600447 +0800
> @@ -12,6 +12,7 @@
>  #include <linux/sched.h>
>  #include <linux/errno.h>
>  #include <asm/processor.h>
> +#include <asm-generic/uaccess.h>
>  
>  #define VERIFY_READ    0
>  #define VERIFY_WRITE   1
> diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h
> --- linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h 2004-06-15 13:41:39.000000000 +0800
> +++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h  2004-06-17 14:51:42.021577010 +0800
> @@ -16,6 +16,7 @@
>   */
>  #include <linux/sched.h>
>  #include <linux/errno.h>
> +#include <asm-generic/uaccess.h>
>  
>  #define VERIFY_READ     0
>  #define VERIFY_WRITE    1

ppc64 and s390 both don't care about alignment, so simply using

#define __get_user_unaligned __get_user
#define __put_user_unaligned __put_user

would make more sense here. See include/asm-*/unaligned.h

> diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h
> --- linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h       2004-06-15 13:41:34.000000000 +0800
> +++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h        2004-06-17 14:51:42.022553572 +0800
> @@ -10,6 +10,7 @@
>  #include <linux/sched.h>
>  #include <linux/prefetch.h>
>  #include <asm/page.h>
> +#include <asm-generic/uaccess.h>
>  
>  #define VERIFY_READ 0
>  #define VERIFY_WRITE 1

Same on x86_64.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

On 6/17/2004 4:36 PM, Arnd Bergmann wrote:

> On Freitag, 18. Juni 2004 00:28, Arun Sharma wrote:
> 
>>It took us a bit longer :) But here's the promised patch. Using
>>__put_user_unaligned() on ia64 may still cause unaligned faults,
>>but we chose to optimize for the common case, where it's 4 byte
>>aligned.   
> 
> 
> Isn't it legal for i386 code to pass syscall arguments with
> an arbitrary alignment? I guess gcc normally aligns everything
> to 32 bit unless you force it not to but you might still return
> error for syscalls that work fine on a native i386 system.
> 

We don't return error, we just take an unaligned fault for this case.

> 
>>+ */
>>+#define __get_user_unaligned(x, ptr)                                   \
>>+({                                                                     \
>>+       __typeof__ (*(ptr)) __x = (x);                                  \
>>+       copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;      \
>>+})
> 
> 
> This should be __copy_from_user instead of copy_from_user, right?
> 

Yes, will fix.

> Under what circumstances would you need to break down a 4 byte
> alignment into 2 bytes? I can understand that you want to optimize
> the unaligned-64 bit case because i386-gcc aligns them only to 32 bit
> in user space, but the other special cases are bogus. Why not
> just do:

I agree that it's unlikely to be used, but I put it in there for completeness. For eg: <asm-ia64/unaligned.h> is doing this too.

> 
> ppc64 and s390 both don't care about alignment, so simply using
> 
> #define __get_user_unaligned __get_user
> #define __put_user_unaligned __put_user
> 
> would make more sense here. See include/asm-*/unaligned.h
> 
> 
>>diff -purN -X dontdiff linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h
>>--- linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h       2004-06-15 13:41:34.000000000 +0800
>>+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h        2004-06-17 14:51:42.022553572 +0800
>>@@ -10,6 +10,7 @@
>> #include <linux/sched.h>
>> #include <linux/prefetch.h>
>> #include <asm/page.h>
>>+#include <asm-generic/uaccess.h>
>> 
>> #define VERIFY_READ 0
>> #define VERIFY_WRITE 1
> 
> 
> Same on x86_64.

Will update the patch for other archs as well.

	-Arun

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

[-- Attachment #1: Type: text/plain, Size: 171 bytes --]

On 6/17/2004 5:56 PM, Arun Sharma wrote:

>>
>> This should be __copy_from_user instead of copy_from_user, right?
>>
> 
> Yes, will fix.

Updated patch attached.

	-Arun


[-- Attachment #2: __get_put_user_unaligned.2.patch --]
[-- Type: text/plain, Size: 7919 bytes --]

(1) Fix compat_filldir64() which is broken on big-endian machines.
(2) Introduce new APIs __{get,put}_user_unaligned.
(3) Optimize __{get,put}_user_unaligned for ia64, x86-64, s390, ppc64.

Thanks to Arnd Bergmann <arnd@arndb.de> for his help.

Signed-off-by: Gordon Jin <gordon.jin@intel.com>
Signed-off-by: Arun Sharma <arun.sharma@intel.com>

diff -purN linux-2.6.7-rc3-getdents/fs/compat.c linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c
--- linux-2.6.7-rc3-getdents/fs/compat.c	2004-06-18 18:41:36.072597378 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c	2004-06-17 14:51:42.000000000 +0800
@@ -979,19 +979,14 @@ static int compat_filldir64(void * __buf
 	dirent = buf->previous;

 	if (dirent) {
-		if (__put_user(offset, (u32 __user *)&dirent->d_off))
-			goto efault;
-		if (__put_user(offset >> 32,
-				((u32 __user *)&dirent->d_off) + 1))
+		if (__put_user_unaligned(offset, &dirent->d_off))
 			goto efault;
 	}
 	dirent = buf->current_dir;
-	if ((__put_user(ino, (u32 __user *)&dirent->d_ino))
-	 || (__put_user(ino >> 32, ((u32 __user *)&dirent->d_ino) + 1)))
+	if (__put_user_unaligned(ino, &dirent->d_ino))
 		goto efault;
 	off = 0;
-	if ((__put_user(off, (u32 __user *)&dirent->d_off))
-	 || (__put_user(off >> 32, ((u32 __user *)&dirent->d_off) + 1)))
+	if (__put_user_unaligned(off, &dirent->d_off))
 		goto efault;
 	if (__put_user(reclen, &dirent->d_reclen))
 		goto efault;
@@ -1040,8 +1035,7 @@ asmlinkage long compat_sys_getdents64(un
 	lastdirent = buf.previous;
 	if (lastdirent) {
 		typeof(lastdirent->d_off) d_off = file->f_pos;
-		__put_user(d_off, (u32 __user *)&lastdirent->d_off);
-		__put_user(d_off >> 32, ((u32 __user *)&lastdirent->d_off) + 1);
+		__put_user_unaligned(d_off, &lastdirent->d_off);
 		error = count - buf.count;
 	}

diff -purN linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h	2004-06-18 18:20:19.274761456 +0800
@@ -0,0 +1,25 @@
+#ifndef _ASM_GENERIC_UACCESS_H_
+#define _ASM_GENERIC_UACCESS_H_
+
+/*
+ * This macro should be used instead of __get_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __get_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x = (x);					\
+	__copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;	\
+})
+
+
+/*
+ * This macro should be used instead of __put_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __put_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x = (x);					\
+	__copy_to_user((ptr), &__x, sizeof(*(ptr))) ? -EFAULT : 0;	\
+})
+
+#endif /* _ASM_GENERIC_UACCESS_H */
diff -purN linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h	2004-06-18 18:41:09.208339895 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h	2004-06-18 18:24:27.561867790 +0800
@@ -91,6 +91,42 @@ verify_area (int type, const void *addr,
 #define __put_user(x, ptr)	__put_user_nocheck((__typeof__(*(ptr))) (x), (ptr), sizeof(*(ptr)))
 #define __get_user(x, ptr)	__get_user_nocheck((x), (ptr), sizeof(*(ptr)))

+extern long __put_user_unaligned_unknown (void);
+
+#define __put_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __put_user((x), (ptr)); break;					\
+		case 2: __ret = (__put_user((x), (u8 __user *)(ptr)))				\
+			| (__put_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__put_user((x), (u16 __user *)(ptr)))				\
+			| (__put_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__put_user((x), (u32 __user *)(ptr)))				\
+			| (__put_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __put_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
+extern long __get_user_unaligned_unknown (void);
+
+#define __get_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __get_user((x), (ptr)); break;					\
+		case 2: __ret = (__get_user((x), (u8 __user *)(ptr)))				\
+			| (__get_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__get_user((x), (u16 __user *)(ptr)))				\
+			| (__get_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__get_user((x), (u32 __user *)(ptr)))				\
+			| (__get_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __get_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
 #ifdef ASM_SUPPORTED
   struct __large_struct { unsigned long buf[100]; };
 # define __m(x) (*(struct __large_struct *)(x))
diff -purN linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h	2004-06-18 18:41:06.639980551 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h	2004-06-17 14:51:42.000000000 +0800
@@ -13,6 +13,7 @@
 #include <linux/compiler.h>
 #include <linux/errno.h>
 #include <linux/thread_info.h>
+#include <asm-generic/uaccess.h>

 /*
  * The fs value determines whether argument validity checking should be
diff -purN linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h	2004-06-18 18:41:04.989589946 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h	2004-06-18 18:27:57.600927717 +0800
@@ -111,6 +111,9 @@ extern unsigned long search_exception_ta
 #define __put_user(x,ptr) \
   __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))

+#define __get_user_unaligned __get_user
+#define __put_user_unaligned __put_user
+
 extern long __put_user_bad(void);

 #define __put_user_nocheck(x,ptr,size)				\
diff -purN linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h	2004-06-18 18:41:20.317714758 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h	2004-06-18 18:31:37.594089084 +0800
@@ -230,6 +230,9 @@ extern int __put_user_bad(void);

 extern int __get_user_bad(void);

+#define __put_user_unaligned __put_user
+#define __get_user_unaligned __get_user
+
 extern long __copy_to_user_asm(const void *from, long n, void *to);

 /**
diff -purN linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h	2004-06-18 18:41:21.579433493 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h	2004-06-17 14:51:42.000000000 +0800
@@ -14,6 +14,7 @@
 #include <asm/asi.h>
 #include <asm/system.h>
 #include <asm/spitfire.h>
+#include <asm-generic/uaccess.h>
 #endif

 #ifndef __ASSEMBLY__
diff -purN linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h	2004-06-18 18:41:04.255214955 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h	2004-06-18 18:32:15.028658938 +0800
@@ -147,6 +147,9 @@ extern void __put_user_bad(void);
 #define __put_user(x,ptr) \
   __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))

+#define __get_user_unaligned __get_user
+#define __put_user_unaligned __put_user
+
 #define __put_user_nocheck(x,ptr,size)			\
 ({							\
 	int __pu_err;					\

Re: sys getdents64 needs compat wrapper ?

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On Freitag, 18. Juni 2004 19:05, Arun Sharma wrote:
> Updated patch attached.

I now noticed one more bug:

> + * values at locations that are unknown to be aligned.
> + */
> +#define __get_user_unaligned(x, ptr)                                   \
> +({                                                                     \
> +       __typeof__ (*(ptr)) __x = (x);                                  \
> +       __copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;    \
> +})

The assignment is done in the wrong direction, this needs to become
something like:

#define __get_user_unaligned(x, ptr)                                   \
({                                                                     \
       __typeof__ (*(ptr)) __x;                                        \
       __copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;    \
       (x) = __x;                                                      \
})

Hopefully that'll be the last change for this, the rest looks good to
me.

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: sys getdents64 needs compat wrapper ?

[Arun Sharma]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On 6/20/2004 2:50 PM, Arnd Bergmann wrote:

> I now noticed one more bug:
> 
>> + * values at locations that are unknown to be aligned.
>> + */
>> +#define __get_user_unaligned(x, ptr)                                   \
>> +({                                                                     \
>> +       __typeof__ (*(ptr)) __x = (x);                                  \
>> +       __copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;    \
>> +})
> 
> The assignment is done in the wrong direction, this needs to become
> something like:
> 
> #define __get_user_unaligned(x, ptr)                                   \
> ({                                                                     \
>        __typeof__ (*(ptr)) __x;                                        \
>        __copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;    \
>        (x) = __x;                                                      \
> })
> 
> Hopefully that'll be the last change for this, the rest looks good to
> me.

Hope so! Take 3.

	-Arun


[-- Attachment #2: __get_put_user_unaligned.3.patch --]
[-- Type: text/plain, Size: 7893 bytes --]

(1) Fix compat_filldir64() which is broken on big-endian machines.
(2) Introduce new APIs __{get,put}_user_unaligned.
(3) Optimize __{get,put}_user_unaligned for ia64, x86-64, ppc64 and s390.

Signed-off-by: Gordon Jin <gordon.jin@intel.com>
Signed-off-by: Arun Sharma <arun.sharma@intel.com>

diff -purN linux-2.6.7-rc3-getdents/fs/compat.c linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c
--- linux-2.6.7-rc3-getdents/fs/compat.c	2004-06-18 18:41:36.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/fs/compat.c	2004-06-17 14:51:42.000000000 +0800
@@ -979,19 +979,14 @@ static int compat_filldir64(void * __buf
 	dirent = buf->previous;
 
 	if (dirent) {
-		if (__put_user(offset, (u32 __user *)&dirent->d_off))
-			goto efault;
-		if (__put_user(offset >> 32,
-				((u32 __user *)&dirent->d_off) + 1))
+		if (__put_user_unaligned(offset, &dirent->d_off))
 			goto efault;
 	}
 	dirent = buf->current_dir;
-	if ((__put_user(ino, (u32 __user *)&dirent->d_ino))
-	 || (__put_user(ino >> 32, ((u32 __user *)&dirent->d_ino) + 1)))
+	if (__put_user_unaligned(ino, &dirent->d_ino))
 		goto efault;
 	off = 0;
-	if ((__put_user(off, (u32 __user *)&dirent->d_off))
-	 || (__put_user(off >> 32, ((u32 __user *)&dirent->d_off) + 1)))
+	if (__put_user_unaligned(off, &dirent->d_off))
 		goto efault;
 	if (__put_user(reclen, &dirent->d_reclen))
 		goto efault;
@@ -1040,8 +1035,7 @@ asmlinkage long compat_sys_getdents64(un
 	lastdirent = buf.previous;
 	if (lastdirent) {
 		typeof(lastdirent->d_off) d_off = file->f_pos;
-		__put_user(d_off, (u32 __user *)&lastdirent->d_off);
-		__put_user(d_off >> 32, ((u32 __user *)&lastdirent->d_off) + 1);
+		__put_user_unaligned(d_off, &lastdirent->d_off);
 		error = count - buf.count;
 	}
 
diff -purN linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-generic/uaccess.h	1970-01-01 08:00:00.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-generic/uaccess.h	2004-06-22 09:40:05.861305422 +0800
@@ -0,0 +1,26 @@
+#ifndef _ASM_GENERIC_UACCESS_H_
+#define _ASM_GENERIC_UACCESS_H_
+
+/*
+ * This macro should be used instead of __get_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __get_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x;					\
+	__copy_from_user(&__x, (ptr), sizeof(*(ptr))) ? -EFAULT : 0;	\
+	(x) = __x;							\
+})
+
+
+/*
+ * This macro should be used instead of __put_user() when accessing
+ * values at locations that are unknown to be aligned.
+ */
+#define __put_user_unaligned(x, ptr)					\
+({									\
+	__typeof__ (*(ptr)) __x = (x);					\
+	__copy_to_user((ptr), &__x, sizeof(*(ptr))) ? -EFAULT : 0;	\
+})
+
+#endif /* _ASM_GENERIC_UACCESS_H */
diff -purN linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ia64/uaccess.h	2004-06-18 18:41:09.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ia64/uaccess.h	2004-06-18 18:24:27.000000000 +0800
@@ -91,6 +91,42 @@ verify_area (int type, const void *addr,
 #define __put_user(x, ptr)	__put_user_nocheck((__typeof__(*(ptr))) (x), (ptr), sizeof(*(ptr)))
 #define __get_user(x, ptr)	__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
 
+extern long __put_user_unaligned_unknown (void);
+
+#define __put_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __put_user((x), (ptr)); break;					\
+		case 2: __ret = (__put_user((x), (u8 __user *)(ptr)))				\
+			| (__put_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__put_user((x), (u16 __user *)(ptr)))				\
+			| (__put_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__put_user((x), (u32 __user *)(ptr)))				\
+			| (__put_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __put_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
+extern long __get_user_unaligned_unknown (void);
+
+#define __get_user_unaligned(x, ptr)								\
+({												\
+	long __ret;										\
+	switch (sizeof(*(ptr))) {								\
+		case 1: __ret = __get_user((x), (ptr)); break;					\
+		case 2: __ret = (__get_user((x), (u8 __user *)(ptr)))				\
+			| (__get_user((x) >> 8, ((u8 __user *)(ptr) + 1))); break;		\
+		case 4: __ret = (__get_user((x), (u16 __user *)(ptr)))				\
+			| (__get_user((x) >> 16, ((u16 __user *)(ptr) + 1))); break;		\
+		case 8: __ret = (__get_user((x), (u32 __user *)(ptr)))				\
+			| (__get_user((x) >> 32, ((u32 __user *)(ptr) + 1))); break;		\
+		default: __ret = __get_user_unaligned_unknown();				\
+	}											\
+	__ret;											\
+})
+
 #ifdef ASM_SUPPORTED
   struct __large_struct { unsigned long buf[100]; };
 # define __m(x) (*(struct __large_struct *)(x))
diff -purN linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-mips/uaccess.h	2004-06-18 18:41:06.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-mips/uaccess.h	2004-06-17 14:51:42.000000000 +0800
@@ -13,6 +13,7 @@
 #include <linux/compiler.h>
 #include <linux/errno.h>
 #include <linux/thread_info.h>
+#include <asm-generic/uaccess.h>
 
 /*
  * The fs value determines whether argument validity checking should be
diff -purN linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-ppc64/uaccess.h	2004-06-18 18:41:04.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-ppc64/uaccess.h	2004-06-18 18:27:57.000000000 +0800
@@ -111,6 +111,9 @@ extern unsigned long search_exception_ta
 #define __put_user(x,ptr) \
   __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
 
+#define __get_user_unaligned __get_user
+#define __put_user_unaligned __put_user
+
 extern long __put_user_bad(void);
 
 #define __put_user_nocheck(x,ptr,size)				\
diff -purN linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-s390/uaccess.h	2004-06-18 18:41:20.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-s390/uaccess.h	2004-06-18 18:31:37.000000000 +0800
@@ -230,6 +230,9 @@ extern int __put_user_bad(void);
 
 extern int __get_user_bad(void);
 
+#define __put_user_unaligned __put_user
+#define __get_user_unaligned __get_user
+
 extern long __copy_to_user_asm(const void *from, long n, void *to);
 
 /**
diff -purN linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-sparc64/uaccess.h	2004-06-18 18:41:21.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-sparc64/uaccess.h	2004-06-17 14:51:42.000000000 +0800
@@ -14,6 +14,7 @@
 #include <asm/asi.h>
 #include <asm/system.h>
 #include <asm/spitfire.h>
+#include <asm-generic/uaccess.h>
 #endif
 
 #ifndef __ASSEMBLY__
diff -purN linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h
--- linux-2.6.7-rc3-getdents/include/asm-x86_64/uaccess.h	2004-06-18 18:41:04.000000000 +0800
+++ linux-2.6.7-rc3-getdents-user_unaligned/include/asm-x86_64/uaccess.h	2004-06-18 18:32:15.000000000 +0800
@@ -147,6 +147,9 @@ extern void __put_user_bad(void);
 #define __put_user(x,ptr) \
   __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
 
+#define __get_user_unaligned __get_user
+#define __put_user_unaligned __put_user
+
 #define __put_user_nocheck(x,ptr,size)			\
 ({							\
 	int __pu_err;					\

=?iso-8859-1?Q?Re:_Re:_Re:_sys_getdents64_needs_compat_wrapper_??=

[Arnd Bergmann]

Arun Sharma <arun.sharma@intel.com> schrieb am 05.06.2004, 02:16:09:

> - use put_user instead of copy_to_user for simple data types.

Not that I care too much, but what's the point of this change? A single
__copy_to_user should be both faster and easier to read than two 
seperate __put_user when writing an unaligned 64 bit value.

       Arnd <><

Re: sys getdents64 needs compat wrapper ?

[Andrew Morton]

Arnd Bergmann <arnd@arndb.de> wrote:
>
> Arun Sharma <arun.sharma@intel.com> schrieb am 05.06.2004, 02:16:09:
> 
>  > - use put_user instead of copy_to_user for simple data types.
> 
>  Not that I care too much, but what's the point of this change? A single
>  __copy_to_user should be both faster and easier to read than two 
>  seperate __put_user when writing an unaligned 64 bit value.
> 

Easier to read, yes.  But __put_user() is faster.  It's a single instruction!

Re: sys getdents64 needs compat wrapper ?

[David S. Miller]

On Sat, 5 Jun 2004 11:41:13 -0700
Andrew Morton <akpm@osdl.org> wrote:

> Easier to read, yes.  But __put_user() is faster.  It's a single
> instruction!

It emits a lot of fixup code though, multiply that by the number of
struct members you're copying over and over the entire kernel it begins
to be a non-trivial amount of .text space.

Actually, even the main part is 2 instructions on Sparc.  One to
do the load/store and one to clear the return value to zero for
success.

It would be much better to have a generic way on each platform to
operate on a series of user structure members like this is so
that the code output looks like:

	load/store	[%ptr + 0], %reg0	! struct foo->a
	load/store	[%ptr + 4], %reg1	! struct foo->b
	load/store	[%ptr + 8], %reg2	! struct foo->c
	clr		%retval
exception_label:

That is the optimal solution for these things.  Because right now
we get code like:

	load/store	[%ptr + 0], %reg0	! struct foo->a
	clr		%retval
exception_label1:
	cmp		%retval, 0
	bne		fault_path
	load/store	[%ptr + 4], %reg1	! struct foo->b
	clr		%retval
exception_label2:
	cmp		%retval, 0
	bne		fault_path
	load/store	[%ptr + 8], %reg2	! struct foo->c
	clr		%retval
exception_label3:

But the optimization is terribly hard to do generic, and the
register availability for such tricks is very platform dependent.