input compat stuff

input compat stuff

[Christoph Hellwig]

http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=52658bb685df77f71e97f1b503dee97d27a88b0f;hp=024ac44c701d43f5e2d34bd6a35b2813a36e6010

besides this beeing butt ugly didn't we have a rule about never checking
task flags whether we're a compat process?  And if we're going to allow
it it should be done properly.

Wish someone would review input patches :P

RFC: is_compat_task (Was: Re: input compat stuff)

[Stephen Rothwell]

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

On Tue, 28 Jun 2005 09:27:10 +0200 Christoph Hellwig <hch@lst.de> wrote:
>
> besides this beeing butt ugly didn't we have a rule about never checking
> task flags whether we're a compat process?  And if we're going to allow
> it it should be done properly.

So, I have been thinking about this patch for a while (wondering how badly
flamed I would get when I posted it :-)).  What do you all think?  Is this
a reasonable thing to do?

We also need sometimes to tell if an mm is 32bit, I think?  Any
suggestions?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus/include/asm-ia64/compat.h linus-willy.3/include/asm-ia64/compat.h
--- linus/include/asm-ia64/compat.h	2005-06-27 16:08:06.000000000 +1000
+++ linus-willy.3/include/asm-ia64/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -4,6 +4,11 @@
  * Architecture specific compatibility types
  */
 #include <linux/types.h>
+#include <linux/sched.h>
+
+#include <asm/system.h>
+#include <asm/ptrace.h>
+#include <asm/processor.h>
 
 #define COMPAT_USER_HZ	100
 
@@ -196,4 +201,9 @@
 	return (void __user *) (((regs->r12 & 0xffffffff) & -16) - len);
 }
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return IS_IA32_PROCESS(ia64_task_regs(t));
+}
+
 #endif /* _ASM_IA64_COMPAT_H */
diff -ruN linus/include/asm-mips/compat.h linus-willy.3/include/asm-mips/compat.h
--- linus/include/asm-mips/compat.h	2005-06-27 16:08:07.000000000 +1000
+++ linus-willy.3/include/asm-mips/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -4,7 +4,9 @@
  * Architecture specific compatibility types
  */
 #include <linux/types.h>
+#include <linux/sched.h>
 #include <asm/page.h>
+#include <asm/processor.h>
 
 #define COMPAT_USER_HZ	100
 
@@ -142,4 +144,9 @@
 #define __COMPAT_ENDIAN_SWAP__ 	1
 #endif
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return (t->thread.mflags & MF_ABI_MASK) == MF_O32;
+}
+
 #endif /* _ASM_COMPAT_H */
diff -ruN linus/include/asm-parisc/compat.h linus-willy.3/include/asm-parisc/compat.h
--- linus/include/asm-parisc/compat.h	2005-06-27 16:08:08.000000000 +1000
+++ linus-willy.3/include/asm-parisc/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -142,4 +142,9 @@
 	return (void __user *)regs->gr[30];
 }
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return personality(t->personality) == PER_LINUX32;
+}
+
 #endif /* _ASM_PARISC_COMPAT_H */
diff -ruN linus/include/asm-ppc64/compat.h linus-willy.3/include/asm-ppc64/compat.h
--- linus/include/asm-ppc64/compat.h	2005-06-27 16:08:08.000000000 +1000
+++ linus-willy.3/include/asm-ppc64/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -200,4 +200,9 @@
 	compat_ulong_t __unused6;
 };
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return test_tsk_thread_flag(t, TIF_32BIT);
+}
+
 #endif /* _ASM_PPC64_COMPAT_H */
diff -ruN linus/include/asm-s390/compat.h linus-willy.3/include/asm-s390/compat.h
--- linus/include/asm-s390/compat.h	2005-06-27 16:08:09.000000000 +1000
+++ linus-willy.3/include/asm-s390/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -195,4 +195,10 @@
 	compat_ulong_t __unused1;
 	compat_ulong_t __unused2;
 };
+
+static inline int is_compat_task(struct task_struct *t)
+{
+	return test_tsk_thread_flag(t, TIF_31BIT);
+}
+
 #endif /* _ASM_S390X_COMPAT_H */
diff -ruN linus/include/asm-sparc64/compat.h linus-willy.3/include/asm-sparc64/compat.h
--- linus/include/asm-sparc64/compat.h	2005-06-27 16:08:10.000000000 +1000
+++ linus-willy.3/include/asm-sparc64/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -4,6 +4,7 @@
  * Architecture specific compatibility types
  */
 #include <linux/types.h>
+#include <linux/sched.h>
 
 #define COMPAT_USER_HZ	100
 
@@ -233,4 +234,9 @@
 	unsigned int	__unused2;
 };
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return test_tsk_thread_flag(t, TIF_32BIT);
+}
+
 #endif /* _ASM_SPARC64_COMPAT_H */
diff -ruN linus/include/asm-x86_64/compat.h linus-willy.3/include/asm-x86_64/compat.h
--- linus/include/asm-x86_64/compat.h	2005-06-27 16:08:10.000000000 +1000
+++ linus-willy.3/include/asm-x86_64/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -202,4 +202,9 @@
 	return (void __user *)regs->rsp - len; 
 }
 
+static inline int is_compat_task(struct task_struct *t)
+{
+	return test_tsk_thread_flag(t, TIF_IA32);
+}
+
 #endif /* _ASM_X86_64_COMPAT_H */
diff -ruN linus/include/linux/compat.h linus-willy.3/include/linux/compat.h
--- linus/include/linux/compat.h	2005-06-27 16:08:11.000000000 +1000
+++ linus-willy.3/include/linux/compat.h	2005-06-27 18:06:35.000000000 +1000
@@ -158,5 +158,9 @@
 int get_compat_sigevent(struct sigevent *event,
 		const struct compat_sigevent __user *u_event);
 
+#else /* CONFIG_COMPAT */
+
+#define is_compat_task(x)	0
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */

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

Re: RFC: is_compat_task

[David S. Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 28 Jun 2005 18:14:53 +1000

> So, I have been thinking about this patch for a while (wondering how badly
> flamed I would get when I posted it :-)).  What do you all think?  Is this
> a reasonable thing to do?

I'm personally not going to fight this any longer, unless
someone can quickly come up with a clean way to handle the
input layer compat stuff without such an interface. :-)

> We also need sometimes to tell if an mm is 32bit, I think?  Any
> suggestions?

Probably time to bite the bullet and put address space ranges
into the mm_struct.

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Andi Kleen]

On Tue, Jun 28, 2005 at 06:14:53PM +1000, Stephen Rothwell wrote:
> On Tue, 28 Jun 2005 09:27:10 +0200 Christoph Hellwig <hch@lst.de> wrote:
> >
> > besides this beeing butt ugly didn't we have a rule about never checking
> > task flags whether we're a compat process?  And if we're going to allow
> > it it should be done properly.
> 
> So, I have been thinking about this patch for a while (wondering how badly
> flamed I would get when I posted it :-)).  What do you all think?  Is this
> a reasonable thing to do?

No, it's not because it makes it impossible to have 64bit processes
that run with 32bit ABI (not implemented right now but we don't
want to break that).

Please don't do this. It is absolutely the wrong thing to do.

-Andi

Re: RFC: is_compat_task

[David S. Miller]

From: Andi Kleen <ak@suse.de>
Date: Tue, 28 Jun 2005 11:17:04 +0200

> No, it's not because it makes it impossible to have 64bit processes
> that run with 32bit ABI (not implemented right now but we don't
> want to break that).

I've been hearing this for years, and still there's nothing
that really wants to do this.

The one case that emulates 32-bit programs from a 64-bit
process (ia64) makes 64-bit system calls.

We can't sit on our hands forever on this issue because of
some pseudo-feature that's been in perpetual beta since the
first time it was ever mentioned :-)

Re: RFC: is_compat_task

[Andi Kleen]

On Tue, Jun 28, 2005 at 02:23:46AM -0700, David S. Miller wrote:
> From: Andi Kleen <ak@suse.de>
> Date: Tue, 28 Jun 2005 11:17:04 +0200
> 
> > No, it's not because it makes it impossible to have 64bit processes
> > that run with 32bit ABI (not implemented right now but we don't
> > want to break that).
> 
> I've been hearing this for years, and still there's nothing
> that really wants to do this.


People regularly ask for 32bit in long mode on x86-64.
So far nobody did really implement it, but I would assume
it will happen sooner or later.

> 
> The one case that emulates 32-bit programs from a 64-bit
> process (ia64) makes 64-bit system calls.
> 
> We can't sit on our hands forever on this issue because of
> some pseudo-feature that's been in perpetual beta since the
> first time it was ever mentioned :-)

On what issue exactly? So far all the code was relatively easily
fixable. 

-Andi

Re: RFC: is_compat_task

[David S. Miller]

From: Andi Kleen <ak@suse.de>
Date: Tue, 28 Jun 2005 11:27:04 +0200

> On what issue exactly? So far all the code was relatively easily
> fixable. 

If this input layer case is too then the situation is different.  But
I suspect this input case is hard to implement without the
is_compat_task() thing.

Prove me wrong and then there's nothing to discuss :)

Re: RFC: is_compat_task

[Andi Kleen]

On Tue, Jun 28, 2005 at 02:29:20AM -0700, David S. Miller wrote:
> From: Andi Kleen <ak@suse.de>
> Date: Tue, 28 Jun 2005 11:27:04 +0200
> 
> > On what issue exactly? So far all the code was relatively easily
> > fixable. 
> 
> If this input layer case is too then the situation is different.  But
> I suspect this input case is hard to implement without the
> is_compat_task() thing.
> 
> Prove me wrong and then there's nothing to discuss :)

I suspect we would need a compat_read/write vector in VFS for this.

-Andi

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Matthew Wilcox]

On Tue, Jun 28, 2005 at 11:17:04AM +0200, Andi Kleen wrote:
> No, it's not because it makes it impossible to have 64bit processes
> that run with 32bit ABI (not implemented right now but we don't
> want to break that).
> 
> Please don't do this. It is absolutely the wrong thing to do.

So.  You want to deliver a signal to a task.  How do you know whether
to send it a 64-bit signal or a 32-bit signal?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Andi Kleen]

On Tue, Jun 28, 2005 at 12:18:28PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 28, 2005 at 11:17:04AM +0200, Andi Kleen wrote:
> > No, it's not because it makes it impossible to have 64bit processes
> > that run with 32bit ABI (not implemented right now but we don't
> > want to break that).
> > 
> > Please don't do this. It is absolutely the wrong thing to do.
> 
> So.  You want to deliver a signal to a task.  How do you know whether
> to send it a 64-bit signal or a 32-bit signal?

For the 32bit-on-64bit case?  You always send 64bit.  Either the
ABI defines 64bit signal data structures or some monitor (like
valgrind or qemu) catches and converts signals themselves.
For the ELF32 case you can test.

-Andi

Re: RFC: is_compat_task

[David S. Miller]

From: Matthew Wilcox <matthew@wil.cx>
Subject: Re: RFC: is_compat_task (Was: Re: input compat stuff)
Date: Tue, 28 Jun 2005 12:18:28 +0100

> On Tue, Jun 28, 2005 at 11:17:04AM +0200, Andi Kleen wrote:
> > No, it's not because it makes it impossible to have 64bit processes
> > that run with 32bit ABI (not implemented right now but we don't
> > want to break that).
> > 
> > Please don't do this. It is absolutely the wrong thing to do.
> 
> So.  You want to deliver a signal to a task.  How do you know whether
> to send it a 64-bit signal or a 32-bit signal?

There is a difference between syscal ABI and execution ABI.
I think Andi is kind of right about this.

Why don't we do the following to resolve this issue?
Create a "is_compat_syscall()" macro, in compat syscall
trap entry, the bit is set, and it is cleared otherwise.

It nops out to always evaluate to "0" on non-compat platforms.

That should resolve all of the concerns, I think.

Re: RFC: is_compat_task

[Stephen Rothwell]

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

On Tue, 28 Jun 2005 13:47:36 -0700 (PDT) "David S. Miller" <davem@davemloft.net> wrote:
>
> Why don't we do the following to resolve this issue?
> Create a "is_compat_syscall()" macro, in compat syscall
> trap entry, the bit is set, and it is cleared otherwise.
> 
> It nops out to always evaluate to "0" on non-compat platforms.

How about the patch below which defines is_compat_syscall() for ppc64 (and
all the non-compat-requiring archs)?  (Yes, the ppc64 syscall entry really
checks the thread flag.) The patch alos includes the fixes for the input
layer mess.

If this is acceptable, I will try to figure out what the macro should be
for the other 64 bit architectures.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus/drivers/input/evdev.c linus-compat_syscall.2/drivers/input/evdev.c
--- linus/drivers/input/evdev.c	2005-06-28 10:05:26.000000000 +1000
+++ linus-compat_syscall.2/drivers/input/evdev.c	2005-06-29 16:00:58.000000000 +1000
@@ -154,16 +154,6 @@
 	__s32 value;
 };
 
-#ifdef CONFIG_X86_64
-#  define COMPAT_TEST test_thread_flag(TIF_IA32)
-#elif defined(CONFIG_IA64)
-#  define COMPAT_TEST IS_IA32_PROCESS(ia64_task_regs(current))
-#elif defined(CONFIG_ARCH_S390)
-#  define COMPAT_TEST test_thread_flag(TIF_31BIT)
-#else
-#  define COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
-
 static ssize_t evdev_write_compat(struct file * file, const char __user * buffer, size_t count, loff_t *ppos)
 {
 	struct evdev_list *list = file->private_data;
@@ -179,6 +169,9 @@
 
 	return retval;
 }
+#else
+/* prototype to keep gcc quiet */
+ssize_t evdev_write_compat(struct file * file, const char __user * buffer, size_t count, loff_t *ppos);
 #endif
 
 static ssize_t evdev_write(struct file * file, const char __user * buffer, size_t count, loff_t *ppos)
@@ -189,10 +182,8 @@
 
 	if (!list->evdev->exist) return -ENODEV;
 
-#ifdef CONFIG_COMPAT
-	if (COMPAT_TEST)
+	if (is_compat_syscall())
 		return evdev_write_compat(file, buffer, count, ppos);
-#endif
 
 	while (retval < count) {
 
@@ -243,6 +234,9 @@
 
 	return retval;
 }
+#else
+/* prototype to keep gcc quiet */
+ssize_t evdev_read_compat(struct file * file, char __user * buffer, size_t count, loff_t *ppos);
 #endif
 
 static ssize_t evdev_read(struct file * file, char __user * buffer, size_t count, loff_t *ppos)
@@ -250,10 +244,8 @@
 	struct evdev_list *list = file->private_data;
 	int retval;
 
-#ifdef CONFIG_COMPAT
-	if (COMPAT_TEST)
+	if (is_compat_syscall())
 		return evdev_read_compat(file, buffer, count, ppos);
-#endif
 
 	if (count < sizeof(struct input_event))
 		return -EINVAL;
@@ -484,34 +476,18 @@
 
 #ifdef CONFIG_COMPAT
 
-#define BITS_PER_LONG_COMPAT (sizeof(compat_long_t) * 8)
-#define NBITS_COMPAT(x) ((((x)-1)/BITS_PER_LONG_COMPAT)+1)
-#define OFF_COMPAT(x)  ((x)%BITS_PER_LONG_COMPAT)
+#define NBITS_COMPAT(x) ((((x)-1)/BITS_PER_COMPAT_LONG)+1)
+#define OFF_COMPAT(x)  ((x)%BITS_PER_COMPAT_LONG)
 #define BIT_COMPAT(x)  (1UL<<OFF_COMPAT(x))
-#define LONG_COMPAT(x) ((x)/BITS_PER_LONG_COMPAT)
+#define LONG_COMPAT(x) ((x)/BITS_PER_COMPAT_LONG)
 #define test_bit_compat(bit, array) ((array[LONG_COMPAT(bit)] >> OFF_COMPAT(bit)) & 1)
 
-#ifdef __BIG_ENDIAN
 #define bit_to_user(bit, max) \
 do { \
-	int i; \
 	int len = NBITS_COMPAT((max)) * sizeof(compat_long_t); \
 	if (len > _IOC_SIZE(cmd)) len = _IOC_SIZE(cmd); \
-	for (i = 0; i < len / sizeof(compat_long_t); i++) \
-		if (copy_to_user((compat_long_t*) p + i, \
-				 (compat_long_t*) (bit) + i + 1 - ((i % 2) << 1), \
-				 sizeof(compat_long_t))) \
-			return -EFAULT; \
-	return len; \
+	return compat_put_bitmap(p, (bit), len * 8) ? -EFAULT : len; \
 } while (0)
-#else
-#define bit_to_user(bit, max) \
-do { \
-	int len = NBITS_COMPAT((max)) * sizeof(compat_long_t); \
-	if (len > _IOC_SIZE(cmd)) len = _IOC_SIZE(cmd); \
-	return copy_to_user(p, (bit), len) ? -EFAULT : len; \
-} while (0)
-#endif
 
 static long evdev_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -631,6 +607,8 @@
 	}
 	return -EINVAL;
 }
+#else
+#define evdev_ioctl_compat	NULL
 #endif
 
 static struct file_operations evdev_fops = {
@@ -641,9 +619,7 @@
 	.open =		evdev_open,
 	.release =	evdev_release,
 	.unlocked_ioctl = evdev_ioctl,
-#ifdef CONFIG_COMPAT
 	.compat_ioctl =	evdev_ioctl_compat,
-#endif
 	.fasync =	evdev_fasync,
 	.flush =	evdev_flush
 };
diff -ruN linus/include/asm-ppc64/compat.h linus-compat_syscall.2/include/asm-ppc64/compat.h
--- linus/include/asm-ppc64/compat.h	2005-06-27 16:08:08.000000000 +1000
+++ linus-compat_syscall.2/include/asm-ppc64/compat.h	2005-06-29 15:15:27.000000000 +1000
@@ -200,4 +200,6 @@
 	compat_ulong_t __unused6;
 };
 
+#define is_compat_syscall()	test_tsk_thread_flag(current, TIF_32BIT)
+
 #endif /* _ASM_PPC64_COMPAT_H */
diff -ruN linus/include/linux/compat.h linus-compat_syscall.2/include/linux/compat.h
--- linus/include/linux/compat.h	2005-06-27 16:08:11.000000000 +1000
+++ linus-compat_syscall.2/include/linux/compat.h	2005-06-29 16:38:46.000000000 +1000
@@ -158,5 +158,15 @@
 int get_compat_sigevent(struct sigevent *event,
 		const struct compat_sigevent __user *u_event);
 
+#else /* CONFIG_COMPAT */
+
+/*
+ * The is_compat_syscall() macro returns true if the current syscall
+ * was done through the comaptibility ABI.  We define it to be zero on
+ * platforms without a compatibility ABI so that the compiler can remove
+ * compatibility code from those platforms.
+ */
+#define is_compat_syscall()	0
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */

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

Re: RFC: is_compat_task

[Arnd Bergmann]

On Middeweken 29 Juni 2005 08:41, Stephen Rothwell wrote:
> How about the patch below which defines is_compat_syscall() for ppc64 (and
> all the non-compat-requiring archs)?  (Yes, the ppc64 syscall entry really
> checks the thread flag.) The patch alos includes the fixes for the input
> layer mess.

Looks ok to me (as in better than all the previous alternatives), though I've
been wondering what is_compat_task() should return outside of syscall context,
e.g. in the exception path or in softirq. I guess the safe choice here would
be to BUG(), but maybe it's just not important.

> If this is acceptable, I will try to figure out what the macro should be
> for the other 64 bit architectures.

Martin Schwidefsky is on holidays this week, so he won't be able to comment
on the s390 part. Testing TIF_31BIT should be the right thing to do there,
for a reason similar to what you wrote about ppc64. The s390 entry code checks
the addressing mode of the user task, which is always set together with the
TIF bit.

	Arnd <><

Re: RFC: is_compat_task

[Andi Kleen]

On Wed, Jun 29, 2005 at 04:41:27PM +1000, Stephen Rothwell wrote:
> On Tue, 28 Jun 2005 13:47:36 -0700 (PDT) "David S. Miller" <davem@davemloft.net> wrote:
> >
> > Why don't we do the following to resolve this issue?
> > Create a "is_compat_syscall()" macro, in compat syscall
> > trap entry, the bit is set, and it is cleared otherwise.
> > 
> > It nops out to always evaluate to "0" on non-compat platforms.
> 
> How about the patch below which defines is_compat_syscall() for ppc64 (and
> all the non-compat-requiring archs)?  (Yes, the ppc64 syscall entry really
> checks the thread flag.) The patch alos includes the fixes for the input
> layer mess.
> for the other 64 bit architectures.

I think for the particular input case it would be still better
to just define ->compat_write at the VFS level.


This would require a new compat_sys_write() wrapper, but that would
be straight forward.

-Andi

Re: RFC: is_compat_task

[Stephen Rothwell]

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

On Wed, 29 Jun 2005 14:12:28 +0200 Andi Kleen <ak@suse.de> wrote:
>
> I think for the particular input case it would be still better
> to just define ->compat_write at the VFS level.
> 
> This would require a new compat_sys_write() wrapper, but that would
> be straight forward.

You mean something like this?

diff -ruN linus/fs/compat.c linus-compat_write.1/fs/compat.c
--- linus/fs/compat.c	2005-06-27 16:08:02.000000000 +1000
+++ linus-compat_write.1/fs/compat.c	2005-06-30 17:48:35.000000000 +1000
@@ -51,6 +51,12 @@
 #include <asm/mmu_context.h>
 #include <asm/ioctls.h>
 
+asmlinkage ssize_t compat_sys_write(unsigned int fd, const char __user * buf,
+		compat_size_t count)
+{
+	return do_sys_write(fd, buf, count, 1);
+}
+
 /*
  * Not all architectures have sys_utime, so implement this in terms
  * of sys_utimes.
diff -ruN linus/fs/read_write.c linus-compat_write.1/fs/read_write.c
--- linus/fs/read_write.c	2005-06-27 16:08:05.000000000 +1000
+++ linus-compat_write.1/fs/read_write.c	2005-06-30 17:44:46.000000000 +1000
@@ -14,6 +14,7 @@
 #include <linux/security.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
+#include <linux/compat.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -283,13 +284,15 @@
 
 EXPORT_SYMBOL(do_sync_write);
 
-ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
+static ssize_t do_vfs_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *pos, int compat)
 {
 	ssize_t ret;
 
 	if (!(file->f_mode & FMODE_WRITE))
 		return -EBADF;
-	if (!file->f_op || (!file->f_op->write && !file->f_op->aio_write))
+	if (!file->f_op || (!file->f_op->write && !file->f_op->aio_write &&
+			!is_compat_write(compat, file->f_op->compat_write)))
 		return -EINVAL;
 	if (unlikely(!access_ok(VERIFY_READ, buf, count)))
 		return -EFAULT;
@@ -298,7 +301,10 @@
 	if (!ret) {
 		ret = security_file_permission (file, MAY_WRITE);
 		if (!ret) {
-			if (file->f_op->write)
+			if (is_compat_write(compat, file->f_op->compat_write))
+				ret = file->f_op->compat_write(file, buf,
+						count, pos);
+			else if (file->f_op->write)
 				ret = file->f_op->write(file, buf, count, pos);
 			else
 				ret = do_sync_write(file, buf, count, pos);
@@ -313,6 +319,12 @@
 	return ret;
 }
 
+ssize_t vfs_write(struct file *file, const char __user *buf, size_t count,
+		loff_t *pos)
+{
+	return do_vfs_write(file, buf, count, pos, 0);
+}
+
 EXPORT_SYMBOL(vfs_write);
 
 static inline loff_t file_pos_read(struct file *file)
@@ -343,7 +355,8 @@
 }
 EXPORT_SYMBOL_GPL(sys_read);
 
-asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count)
+ssize_t do_sys_write(unsigned int fd, const char __user * buf, size_t count,
+		int compat)
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
@@ -352,7 +365,7 @@
 	file = fget_light(fd, &fput_needed);
 	if (file) {
 		loff_t pos = file_pos_read(file);
-		ret = vfs_write(file, buf, count, &pos);
+		ret = do_vfs_write(file, buf, count, &pos, compat);
 		file_pos_write(file, pos);
 		fput_light(file, fput_needed);
 	}
@@ -360,6 +373,12 @@
 	return ret;
 }
 
+asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf,
+		size_t count)
+{
+	return do_sys_write(fd, buf, count, 0);
+}
+
 asmlinkage ssize_t sys_pread64(unsigned int fd, char __user *buf,
 			     size_t count, loff_t pos)
 {
diff -ruN linus/include/linux/compat.h linus-compat_write.1/include/linux/compat.h
--- linus/include/linux/compat.h	2005-06-27 16:08:11.000000000 +1000
+++ linus-compat_write.1/include/linux/compat.h	2005-06-30 17:52:59.000000000 +1000
@@ -158,5 +158,11 @@
 int get_compat_sigevent(struct sigevent *event,
 		const struct compat_sigevent __user *u_event);
 
+#define is_compat_write(flag, func)	((flag) && (NULL != func))
+
+#else /* CONFIG_COMPAT */
+
+#define is_compat_write(flag, func)	0
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
diff -ruN linus/include/linux/fs.h linus-compat_write.1/include/linux/fs.h
--- linus/include/linux/fs.h	2005-06-28 10:05:27.000000000 +1000
+++ linus-compat_write.1/include/linux/fs.h	2005-06-30 17:50:59.000000000 +1000
@@ -951,6 +951,7 @@
 	ssize_t (*aio_read) (struct kiocb *, char __user *, size_t, loff_t);
 	ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
 	ssize_t (*aio_write) (struct kiocb *, const char __user *, size_t, loff_t);
+	ssize_t (*compat_write) (struct file *, const char __user *, size_t, loff_t *);
 	int (*readdir) (struct file *, void *, filldir_t);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long);
@@ -1498,6 +1499,8 @@
 		unsigned long, loff_t, loff_t *, size_t, ssize_t);
 extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
 extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
+extern ssize_t do_sys_write(unsigned int fd, const char __user * buf,
+		size_t count, int compat);
 ssize_t generic_file_write_nolock(struct file *file, const struct iovec *iov,
 				unsigned long nr_segs, loff_t *ppos);
 extern ssize_t generic_file_sendfile(struct file *, loff_t *, size_t, read_actor_t, void *);

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: RFC: is_compat_task

[Andi Kleen]

On Thu, Jun 30, 2005 at 05:57:41PM +1000, Stephen Rothwell wrote:
> On Wed, 29 Jun 2005 14:12:28 +0200 Andi Kleen <ak@suse.de> wrote:
> >
> > I think for the particular input case it would be still better
> > to just define ->compat_write at the VFS level.
> > 
> > This would require a new compat_sys_write() wrapper, but that would
> > be straight forward.
> 
> You mean something like this?

Yes, but ...

>  
> -ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
> +static ssize_t do_vfs_write(struct file *file, const char __user *buf,
> +		size_t count, loff_t *pos, int compat)

Instead of the flag I would move the code figuring out the call vector
into the caller.

-Andi

Re: RFC: is_compat_task

[Stephen Rothwell]

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

On Thu, 30 Jun 2005 10:42:55 +0200 Andi Kleen <ak@suse.de> wrote:
>
> > -ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
> > +static ssize_t do_vfs_write(struct file *file, const char __user *buf,
> > +		size_t count, loff_t *pos, int compat)
> 
> Instead of the flag I would move the code figuring out the call vector
> into the caller.

Like below?  I am not sure that this is better as we duplicate the code
that selects the write_func ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus/fs/compat.c linus-compat_write.2/fs/compat.c
--- linus/fs/compat.c	2005-06-27 16:08:02.000000000 +1000
+++ linus-compat_write.2/fs/compat.c	2005-06-30 17:48:35.000000000 +1000
@@ -51,6 +51,12 @@
 #include <asm/mmu_context.h>
 #include <asm/ioctls.h>
 
+asmlinkage ssize_t compat_sys_write(unsigned int fd, const char __user * buf,
+		compat_size_t count)
+{
+	return do_sys_write(fd, buf, count, 1);
+}
+
 /*
  * Not all architectures have sys_utime, so implement this in terms
  * of sys_utimes.
diff -ruN linus/fs/read_write.c linus-compat_write.2/fs/read_write.c
--- linus/fs/read_write.c	2005-06-27 16:08:05.000000000 +1000
+++ linus-compat_write.2/fs/read_write.c	2005-07-01 00:31:08.000000000 +1000
@@ -14,6 +14,7 @@
 #include <linux/security.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
+#include <linux/compat.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -283,13 +284,16 @@
 
 EXPORT_SYMBOL(do_sync_write);
 
-ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
+static ssize_t do_vfs_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *pos,
+		ssize_t (*write_func)(struct file *, const char __user *,
+			size_t, loff_t *))
 {
 	ssize_t ret;
 
 	if (!(file->f_mode & FMODE_WRITE))
 		return -EBADF;
-	if (!file->f_op || (!file->f_op->write && !file->f_op->aio_write))
+	if (!write_func)
 		return -EINVAL;
 	if (unlikely(!access_ok(VERIFY_READ, buf, count)))
 		return -EFAULT;
@@ -298,10 +302,7 @@
 	if (!ret) {
 		ret = security_file_permission (file, MAY_WRITE);
 		if (!ret) {
-			if (file->f_op->write)
-				ret = file->f_op->write(file, buf, count, pos);
-			else
-				ret = do_sync_write(file, buf, count, pos);
+			ret = write_func(file, buf, count, pos);
 			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_MODIFY);
 				current->wchar += ret;
@@ -313,6 +314,21 @@
 	return ret;
 }
 
+ssize_t vfs_write(struct file *file, const char __user *buf, size_t count,
+		loff_t *pos)
+{
+	ssize_t (*write_func)(struct file *, const char __user *,
+			size_t, loff_t *) = NULL;
+
+	if (file->f_ops) {
+		if (file->f_op->write)
+			write_func = file->f_op->write;
+		else if (file->f_op->aio_write)
+			write_func = &do_sync_write;
+	}
+	return do_vfs_write(file, buf, count, pos, write_func);
+}
+
 EXPORT_SYMBOL(vfs_write);
 
 static inline loff_t file_pos_read(struct file *file)
@@ -343,16 +359,27 @@
 }
 EXPORT_SYMBOL_GPL(sys_read);
 
-asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count)
+ssize_t do_sys_write(unsigned int fd, const char __user * buf, size_t count,
+		int compat)
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
 	int fput_needed;
+	ssize_t (*write_func)(struct file *, const char __user *,
+			size_t, loff_t *) = NULL;
 
 	file = fget_light(fd, &fput_needed);
 	if (file) {
 		loff_t pos = file_pos_read(file);
-		ret = vfs_write(file, buf, count, &pos);
+		if (file->f_ops) {
+			if (is_compat_write(compat, file->f_op->compat_write))
+				write_func = file->f_op->compat_write;
+			else if (file->f_op->write)
+				write_func = file->f_op->write;
+			else if (file->f_op->aio_write)
+				write_func = &do_sync_write;
+		}
+		ret = do_vfs_write(file, buf, count, &pos, write_func);
 		file_pos_write(file, pos);
 		fput_light(file, fput_needed);
 	}
@@ -360,6 +387,12 @@
 	return ret;
 }
 
+asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf,
+		size_t count)
+{
+	return do_sys_write(fd, buf, count, 0);
+}
+
 asmlinkage ssize_t sys_pread64(unsigned int fd, char __user *buf,
 			     size_t count, loff_t pos)
 {
diff -ruN linus/include/linux/compat.h linus-compat_write.2/include/linux/compat.h
--- linus/include/linux/compat.h	2005-06-27 16:08:11.000000000 +1000
+++ linus-compat_write.2/include/linux/compat.h	2005-06-30 17:52:59.000000000 +1000
@@ -158,5 +158,11 @@
 int get_compat_sigevent(struct sigevent *event,
 		const struct compat_sigevent __user *u_event);
 
+#define is_compat_write(flag, func)	((flag) && (NULL != func))
+
+#else /* CONFIG_COMPAT */
+
+#define is_compat_write(flag, func)	0
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
diff -ruN linus/include/linux/fs.h linus-compat_write.2/include/linux/fs.h
--- linus/include/linux/fs.h	2005-06-28 10:05:27.000000000 +1000
+++ linus-compat_write.2/include/linux/fs.h	2005-06-30 17:50:59.000000000 +1000
@@ -951,6 +951,7 @@
 	ssize_t (*aio_read) (struct kiocb *, char __user *, size_t, loff_t);
 	ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
 	ssize_t (*aio_write) (struct kiocb *, const char __user *, size_t, loff_t);
+	ssize_t (*compat_write) (struct file *, const char __user *, size_t, loff_t *);
 	int (*readdir) (struct file *, void *, filldir_t);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long);
@@ -1498,6 +1499,8 @@
 		unsigned long, loff_t, loff_t *, size_t, ssize_t);
 extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
 extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
+extern ssize_t do_sys_write(unsigned int fd, const char __user * buf,
+		size_t count, int compat);
 ssize_t generic_file_write_nolock(struct file *file, const struct iovec *iov,
 				unsigned long nr_segs, loff_t *ppos);
 extern ssize_t generic_file_sendfile(struct file *, loff_t *, size_t, read_actor_t, void *);

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

Re: RFC: is_compat_task

[Stephen Rothwell]

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

On Fri, 1 Jul 2005 00:38:06 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Like below?  I am not sure that this is better as we duplicate the code
> that selects the write_func ...

This is seeming like a lot of infrastructure change (I did the read
version as well) for a single (badly done) API.  How about we go with
is_compat_syscall() for 2.6.13 and revisit the VFS changes after that?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: RFC: is_compat_task

[Andi Kleen]

On Fri, Jul 01, 2005 at 01:28:57AM +1000, Stephen Rothwell wrote:
> On Fri, 1 Jul 2005 00:38:06 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Like below?  I am not sure that this is better as we duplicate the code
> > that selects the write_func ...
> 
> This is seeming like a lot of infrastructure change (I did the read
> version as well) for a single (badly done) API.  How about we go with
> is_compat_syscall() for 2.6.13 and revisit the VFS changes after that?

Please just do the infrastructure change. If we start with hacks
they will never come out again.

-Andi

Re: RFC: is_compat_task

[Paul Mackerras]

Andi Kleen writes:

> On Fri, Jul 01, 2005 at 01:28:57AM +1000, Stephen Rothwell wrote:
> > On Fri, 1 Jul 2005 00:38:06 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Like below?  I am not sure that this is better as we duplicate the code
> > > that selects the write_func ...
> > 
> > This is seeming like a lot of infrastructure change (I did the read
> > version as well) for a single (badly done) API.  How about we go with
> > is_compat_syscall() for 2.6.13 and revisit the VFS changes after that?
> 
> Please just do the infrastructure change. If we start with hacks
> they will never come out again.

The problem with that is that it seems horrible to me to interpret the
contents of a buffer being given to write() differently depending on
whether it's a 32-bit or a 64-bit task that is writing it.  The data
format *should* be designed so that it is wordsize-independent (and
preferably endian-independent too), or else it should at least be
self-describing.

Adding this infrastructure is just going to encourage people to do the
wrong thing in future.  I know we already have the input layer doing
the wrong thing, and we need to hack around that, but I think the hack
should be as small and self-contained as possible, rather than adding
a whole infrastructure to support the brokenness.

Regards,
Paul.

Re: RFC: is_compat_task

[David S. Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Fri, 1 Jul 2005 09:29:25 +1000

> The problem with that is that it seems horrible to me to interpret the
> contents of a buffer being given to write() differently depending on
> whether it's a 32-bit or a 64-bit task that is writing it.  The data
> format *should* be designed so that it is wordsize-independent (and
> preferably endian-independent too), or else it should at least be
> self-describing.

Unfortunately, on ia64 and x86_64 executing x86 binaries,
netlink has the same exact problems, particular for IPSEC.

So it's not just the input layer with this problem.

Re: RFC: is_compat_task

[Andi Kleen]

> Adding this infrastructure is just going to encourage people to do the
> wrong thing in future.  I know we already have the input layer doing
> the wrong thing, and we need to hack around that, but I think the hack
> should be as small and self-contained as possible, rather than adding

It won't be. It needs hacks in the system call entry part and
in other critical parts.  

> a whole infrastructure to support the brokenness.

I agree that a big fat comment, something like

"If you need to use this you did something very wrong and should 
be ashamed." (with a bit more explanation) 

on the new vectors would be a good idea.

On the other hand we probably need more Documentation on how
to design data structures to be compat clean. Too many people
get this wrong and it is admittedly a bit of black art.

-Andi

Re: RFC: is_compat_task

[Paul Mackerras]

Andi Kleen writes:

> > Adding this infrastructure is just going to encourage people to do the
> > wrong thing in future.  I know we already have the input layer doing
> > the wrong thing, and we need to hack around that, but I think the hack
> > should be as small and self-contained as possible, rather than adding
> 
> It won't be. It needs hacks in the system call entry part and
> in other critical parts.  

Unfortunately it's not just compat versions of read and write, there
are pread/pwrite, and the various socket send and receive functions.
The number of extra bits of infrastructure we need to add seems to
just keep growing.  On the other hand, with an is_compat_syscall() we
could handle all of those with one piece of infrastructure.

Paul.

Re: RFC: is_compat_task

[David S. Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Mon, 4 Jul 2005 13:36:08 +1000

> Unfortunately it's not just compat versions of read and write, there
> are pread/pwrite, and the various socket send and receive functions.
> The number of extra bits of infrastructure we need to add seems to
> just keep growing.  On the other hand, with an is_compat_syscall() we
> could handle all of those with one piece of infrastructure.

True, it does seem to be a more fruitless path than
finally capitulating to is_compat_syscall() or something
similar.

[PATCH, for review 1/3] compat_sys_{read,write}

[Stephen Rothwell]

This patch introduces compatibility syscalls for read and write and the
equivalent hooks in struct file_operations.  These are neede to support
the badly designed (i.e. broken) input layer evdev read and write
interfaces.

I have built this on ppc64 and booted it on an iSeries machine.

Comments?  Testing?

 fs/compat.c        |   12 ++++++++
 fs/read_write.c    |   77 +++++++++++++++++++++++++++++++++++++++++------------
 include/linux/fs.h |    6 ++++
 3 files changed, 79 insertions(+), 16 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus/fs/compat.c linus-compat_write.3/fs/compat.c
--- linus/fs/compat.c	2005-06-27 16:08:02.000000000 +1000
+++ linus-compat_write.3/fs/compat.c	2005-07-01 13:48:57.000000000 +1000
@@ -51,6 +51,18 @@
 #include <asm/mmu_context.h>
 #include <asm/ioctls.h>
 
+asmlinkage ssize_t compat_sys_read(unsigned int fd, char __user * buf,
+		compat_size_t count)
+{
+	return do_sys_read(fd, buf, count, 1);
+}
+
+asmlinkage ssize_t compat_sys_write(unsigned int fd, const char __user * buf,
+		compat_size_t count)
+{
+	return do_sys_write(fd, buf, count, 1);
+}
+
 /*
  * Not all architectures have sys_utime, so implement this in terms
  * of sys_utimes.
diff -ruN linus/fs/read_write.c linus-compat_write.3/fs/read_write.c
--- linus/fs/read_write.c	2005-06-27 16:08:05.000000000 +1000
+++ linus-compat_write.3/fs/read_write.c	2005-07-01 14:39:04.000000000 +1000
@@ -18,6 +18,26 @@
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
 
+#ifdef CONFIG_COMPAT
+#define vfs_select_rw(file, func, compat)				\
+		(((file)->f_op == NULL) ? NULL				\
+		 : (((compat) && ((file)->f_op->compat_ ## func != NULL)) \
+			? (file)->f_op->compat_ ## func			\
+			: (((file)->f_op->func != NULL)			\
+				? (file)->f_op->func			\
+				: (((file)->f_op->aio_ ## func != NULL)	\
+					? do_sync_ ## func		\
+					: NULL))))
+#else
+#define vfs_select_rw(file, func, compat)				\
+		(((file)->f_op == NULL) ? NULL				\
+			: (((file)->f_op->func != NULL)			\
+				? (file)->f_op->func			\
+				: (((file)->f_op->aio_ ## func != NULL)	\
+					? do_sync_ ## func		\
+					: NULL)))
+#endif	/* CONFIG_COMPAT */
+
 struct file_operations generic_ro_fops = {
 	.llseek		= generic_file_llseek,
 	.read		= generic_file_read,
@@ -232,13 +252,15 @@
 
 EXPORT_SYMBOL(do_sync_read);
 
-ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
+static ssize_t do_vfs_read(struct file *file, char __user *buf, size_t count,
+		loff_t *pos, ssize_t (*read_func)(struct file *, char __user *,
+			size_t, loff_t *))
 {
 	ssize_t ret;
 
 	if (!(file->f_mode & FMODE_READ))
 		return -EBADF;
-	if (!file->f_op || (!file->f_op->read && !file->f_op->aio_read))
+	if (!read_func)
 		return -EINVAL;
 	if (unlikely(!access_ok(VERIFY_WRITE, buf, count)))
 		return -EFAULT;
@@ -247,10 +269,7 @@
 	if (!ret) {
 		ret = security_file_permission (file, MAY_READ);
 		if (!ret) {
-			if (file->f_op->read)
-				ret = file->f_op->read(file, buf, count, pos);
-			else
-				ret = do_sync_read(file, buf, count, pos);
+			ret = read_func(file, buf, count, pos);
 			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_ACCESS);
 				current->rchar += ret;
@@ -262,6 +281,10 @@
 	return ret;
 }
 
+ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
+{
+	return do_vfs_read(file, buf, count, pos, vfs_select_rw(file, read, 0));
+}
 EXPORT_SYMBOL(vfs_read);
 
 ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos)
@@ -283,13 +306,16 @@
 
 EXPORT_SYMBOL(do_sync_write);
 
-ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
+static ssize_t do_vfs_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *pos,
+		ssize_t (*write_func)(struct file *, const char __user *,
+			size_t, loff_t *))
 {
 	ssize_t ret;
 
 	if (!(file->f_mode & FMODE_WRITE))
 		return -EBADF;
-	if (!file->f_op || (!file->f_op->write && !file->f_op->aio_write))
+	if (!write_func)
 		return -EINVAL;
 	if (unlikely(!access_ok(VERIFY_READ, buf, count)))
 		return -EFAULT;
@@ -298,10 +324,7 @@
 	if (!ret) {
 		ret = security_file_permission (file, MAY_WRITE);
 		if (!ret) {
-			if (file->f_op->write)
-				ret = file->f_op->write(file, buf, count, pos);
-			else
-				ret = do_sync_write(file, buf, count, pos);
+			ret = write_func(file, buf, count, pos);
 			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_MODIFY);
 				current->wchar += ret;
@@ -313,6 +336,13 @@
 	return ret;
 }
 
+ssize_t vfs_write(struct file *file, const char __user *buf, size_t count,
+		loff_t *pos)
+{
+	return do_vfs_write(file, buf, count, pos,
+			vfs_select_rw(file, write, 0));
+}
+
 EXPORT_SYMBOL(vfs_write);
 
 static inline loff_t file_pos_read(struct file *file)
@@ -325,7 +355,8 @@
 	file->f_pos = pos;
 }
 
-asmlinkage ssize_t sys_read(unsigned int fd, char __user * buf, size_t count)
+ssize_t do_sys_read(unsigned int fd, char __user * buf,
+		size_t count, int compat)
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
@@ -334,16 +365,23 @@
 	file = fget_light(fd, &fput_needed);
 	if (file) {
 		loff_t pos = file_pos_read(file);
-		ret = vfs_read(file, buf, count, &pos);
+		ret = do_vfs_read(file, buf, count, &pos,
+				vfs_select_rw(file, read, compat));
 		file_pos_write(file, pos);
 		fput_light(file, fput_needed);
 	}
 
 	return ret;
 }
+
+asmlinkage ssize_t sys_read(unsigned int fd, char __user * buf, size_t count)
+{
+	return do_sys_read(fd, buf, count, 0);
+}
 EXPORT_SYMBOL_GPL(sys_read);
 
-asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count)
+ssize_t do_sys_write(unsigned int fd, const char __user * buf, size_t count,
+		int compat)
 {
 	struct file *file;
 	ssize_t ret = -EBADF;
@@ -352,7 +390,8 @@
 	file = fget_light(fd, &fput_needed);
 	if (file) {
 		loff_t pos = file_pos_read(file);
-		ret = vfs_write(file, buf, count, &pos);
+		ret = do_vfs_write(file, buf, count, &pos,
+				vfs_select_rw(file, write, compat));
 		file_pos_write(file, pos);
 		fput_light(file, fput_needed);
 	}
@@ -360,6 +399,12 @@
 	return ret;
 }
 
+asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf,
+		size_t count)
+{
+	return do_sys_write(fd, buf, count, 0);
+}
+
 asmlinkage ssize_t sys_pread64(unsigned int fd, char __user *buf,
 			     size_t count, loff_t pos)
 {
diff -ruN linus/include/linux/fs.h linus-compat_write.3/include/linux/fs.h
--- linus/include/linux/fs.h	2005-06-28 10:05:27.000000000 +1000
+++ linus-compat_write.3/include/linux/fs.h	2005-07-01 13:46:07.000000000 +1000
@@ -949,8 +949,10 @@
 	loff_t (*llseek) (struct file *, loff_t, int);
 	ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
 	ssize_t (*aio_read) (struct kiocb *, char __user *, size_t, loff_t);
+	ssize_t (*compat_read) (struct file *, char __user *, size_t, loff_t *);
 	ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
 	ssize_t (*aio_write) (struct kiocb *, const char __user *, size_t, loff_t);
+	ssize_t (*compat_write) (struct file *, const char __user *, size_t, loff_t *);
 	int (*readdir) (struct file *, void *, filldir_t);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long);
@@ -1498,6 +1500,10 @@
 		unsigned long, loff_t, loff_t *, size_t, ssize_t);
 extern ssize_t do_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
 extern ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos);
+extern ssize_t do_sys_read(unsigned int fd, char __user * buf,
+		size_t count, int compat);
+extern ssize_t do_sys_write(unsigned int fd, const char __user * buf,
+		size_t count, int compat);
 ssize_t generic_file_write_nolock(struct file *file, const struct iovec *iov,
 				unsigned long nr_segs, loff_t *ppos);
 extern ssize_t generic_file_sendfile(struct file *, loff_t *, size_t, read_actor_t, void *);

[PATCH, for review 2/3] hook up compat_sys_{read,write} syscalls

[Stephen Rothwell]

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

This patch just hooks the compat_sys_{read,write} into the appropriate syscall tables.

Please check and comment.
---

 ia64/ia32/ia32_entry.S        |    4 ++--
 mips/kernel/scall64-n32.S     |    4 ++--
 mips/kernel/scall64-o32.S     |    4 ++--
 parisc/kernel/syscall_table.S |    4 ++--
 ppc64/kernel/misc.S           |    4 ++--
 s390/kernel/compat_linux.c    |   16 ----------------
 s390/kernel/compat_wrapper.S  |   12 ++++++------
 s390/kernel/syscalls.S        |    4 ++--
 sparc64/kernel/systbls.S      |    2 +-
 x86_64/ia32/ia32entry.S       |    4 ++--
 10 files changed, 21 insertions(+), 37 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus-compat_write.3/arch/ia64/ia32/ia32_entry.S linus-compat_write.4/arch/ia64/ia32/ia32_entry.S
--- linus-compat_write.3/arch/ia64/ia32/ia32_entry.S	2005-06-27 16:07:59.000000000 +1000
+++ linus-compat_write.4/arch/ia64/ia32/ia32_entry.S	2005-07-01 13:58:03.000000000 +1000
@@ -213,8 +213,8 @@
 	data8 sys_ni_syscall	  /* 0	-  old "setup(" system call*/
 	data8 sys_exit
 	data8 sys32_fork
-	data8 sys_read
-	data8 sys_write
+	data8 compat_sys_read
+	data8 compat_sys_write
 	data8 sys32_open	  /* 5 */
 	data8 sys_close
 	data8 sys32_waitpid
diff -ruN linus-compat_write.3/arch/mips/kernel/scall64-n32.S linus-compat_write.4/arch/mips/kernel/scall64-n32.S
--- linus-compat_write.3/arch/mips/kernel/scall64-n32.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/mips/kernel/scall64-n32.S	2005-07-01 13:59:26.000000000 +1000
@@ -117,8 +117,8 @@
 	END(handle_sysn32)
 
 EXPORT(sysn32_call_table)
-	PTR	sys_read			/* 6000 */
-	PTR	sys_write
+	PTR	compat_sys_read			/* 6000 */
+	PTR	compat_sys_write
 	PTR	sys_open
 	PTR	sys_close
 	PTR	sys_newstat
diff -ruN linus-compat_write.3/arch/mips/kernel/scall64-o32.S linus-compat_write.4/arch/mips/kernel/scall64-o32.S
--- linus-compat_write.3/arch/mips/kernel/scall64-o32.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/mips/kernel/scall64-o32.S	2005-07-01 13:59:40.000000000 +1000
@@ -205,8 +205,8 @@
 	PTR	sys32_syscall			/* 4000 */
 	PTR	sys_exit
 	PTR	sys_fork
-	PTR	sys_read
-	PTR	sys_write
+	PTR	compat_sys_read
+	PTR	compat_sys_write
 	PTR	sys_open			/* 4005 */
 	PTR	sys_close
 	PTR	sys_waitpid
diff -ruN linus-compat_write.3/arch/parisc/kernel/syscall_table.S linus-compat_write.4/arch/parisc/kernel/syscall_table.S
--- linus-compat_write.3/arch/parisc/kernel/syscall_table.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/parisc/kernel/syscall_table.S	2005-07-01 14:01:50.000000000 +1000
@@ -63,8 +63,8 @@
 	ENTRY_SAME(restart_syscall)	/* 0 */
 	ENTRY_SAME(exit)
 	ENTRY_SAME(fork_wrapper)
-	ENTRY_SAME(read)
-	ENTRY_SAME(write)
+	ENTRY_COMP(read)
+	ENTRY_COMP(write)
 	ENTRY_SAME(open)		/* 5 */
 	ENTRY_SAME(close)
 	ENTRY_SAME(waitpid)
diff -ruN linus-compat_write.3/arch/ppc64/kernel/misc.S linus-compat_write.4/arch/ppc64/kernel/misc.S
--- linus-compat_write.3/arch/ppc64/kernel/misc.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/ppc64/kernel/misc.S	2005-07-01 14:02:23.000000000 +1000
@@ -857,8 +857,8 @@
 	.llong .sys_restart_syscall	/* 0 */
 	.llong .sys_exit
 	.llong .ppc_fork
-	.llong .sys_read
-	.llong .sys_write
+	.llong .compat_sys_read
+	.llong .compat_sys_write
 	.llong .sys32_open		/* 5 */
 	.llong .sys_close
 	.llong .sys32_waitpid
diff -ruN linus-compat_write.3/arch/s390/kernel/compat_linux.c linus-compat_write.4/arch/s390/kernel/compat_linux.c
--- linus-compat_write.3/arch/s390/kernel/compat_linux.c	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/s390/kernel/compat_linux.c	2005-07-01 14:06:36.000000000 +1000
@@ -980,22 +980,6 @@
 	return error;
 }
 
-asmlinkage long sys32_read(unsigned int fd, char * buf, size_t count)
-{
-	if ((compat_ssize_t) count < 0)
-		return -EINVAL; 
-
-	return sys_read(fd, buf, count);
-}
-
-asmlinkage long sys32_write(unsigned int fd, char * buf, size_t count)
-{
-	if ((compat_ssize_t) count < 0)
-		return -EINVAL; 
-
-	return sys_write(fd, buf, count);
-}
-
 asmlinkage long sys32_clone(struct pt_regs regs)
 {
         unsigned long clone_flags;
diff -ruN linus-compat_write.3/arch/s390/kernel/compat_wrapper.S linus-compat_write.4/arch/s390/kernel/compat_wrapper.S
--- linus-compat_write.3/arch/s390/kernel/compat_wrapper.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/s390/kernel/compat_wrapper.S	2005-07-01 14:09:52.000000000 +1000
@@ -13,19 +13,19 @@
 	lgfr	%r2,%r2			# int
 	jg	sys_exit		# branch to sys_exit
     
-	.globl  sys32_read_wrapper 
-sys32_read_wrapper:
+	.globl  compat_read_wrapper
+compat_sys_read_wrapper:
 	llgfr	%r2,%r2			# unsigned int
 	llgtr	%r3,%r3			# char *
 	llgfr	%r4,%r4			# size_t
-	jg	sys32_read		# branch to sys_read
+	jg	compat_sys_read		# branch to sys_read
 
-	.globl  sys32_write_wrapper 
-sys32_write_wrapper:
+	.globl  compat_write_wrapper
+compat_sys_write_wrapper:
 	llgfr	%r2,%r2			# unsigned int
 	llgtr	%r3,%r3			# const char *
 	llgfr	%r4,%r4			# size_t
-	jg	sys32_write		# branch to system call
+	jg	compat_sys_write	# branch to system call
 
 	.globl  sys32_open_wrapper 
 sys32_open_wrapper:
diff -ruN linus-compat_write.3/arch/s390/kernel/syscalls.S linus-compat_write.4/arch/s390/kernel/syscalls.S
--- linus-compat_write.3/arch/s390/kernel/syscalls.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/s390/kernel/syscalls.S	2005-07-01 14:10:05.000000000 +1000
@@ -11,8 +11,8 @@
 NI_SYSCALL							/* 0 */
 SYSCALL(sys_exit,sys_exit,sys32_exit_wrapper)
 SYSCALL(sys_fork_glue,sys_fork_glue,sys_fork_glue)
-SYSCALL(sys_read,sys_read,sys32_read_wrapper)
-SYSCALL(sys_write,sys_write,sys32_write_wrapper)
+SYSCALL(sys_read,sys_read,compat_sys_read_wrapper)
+SYSCALL(sys_write,sys_write,compat_sys_write_wrapper)
 SYSCALL(sys_open,sys_open,sys32_open_wrapper)			/* 5 */
 SYSCALL(sys_close,sys_close,sys32_close_wrapper)
 SYSCALL(sys_restart_syscall,sys_restart_syscall,sys_restart_syscall)
diff -ruN linus-compat_write.3/arch/sparc64/kernel/systbls.S linus-compat_write.4/arch/sparc64/kernel/systbls.S
--- linus-compat_write.3/arch/sparc64/kernel/systbls.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/sparc64/kernel/systbls.S	2005-07-01 14:12:24.000000000 +1000
@@ -20,7 +20,7 @@
 
 	.globl sys_call_table32
 sys_call_table32:
-/*0*/	.word sys_restart_syscall, sys32_exit, sys_fork, sys_read, sys_write
+/*0*/	.word sys_restart_syscall, sys32_exit, sys_fork, compat_sys_read, compat_sys_write
 /*5*/	.word sys32_open, sys_close, sys32_wait4, sys32_creat, sys_link
 /*10*/  .word sys_unlink, sunos_execv, sys_chdir, sys32_chown16, sys32_mknod
 /*15*/	.word sys_chmod, sys32_lchown16, sparc_brk, sys32_perfctr, sys32_lseek
diff -ruN linus-compat_write.3/arch/x86_64/ia32/ia32entry.S linus-compat_write.4/arch/x86_64/ia32/ia32entry.S
--- linus-compat_write.3/arch/x86_64/ia32/ia32entry.S	2005-06-27 16:08:00.000000000 +1000
+++ linus-compat_write.4/arch/x86_64/ia32/ia32entry.S	2005-07-01 14:12:50.000000000 +1000
@@ -305,8 +305,8 @@
 	.quad sys_restart_syscall
 	.quad sys_exit
 	.quad stub32_fork
-	.quad sys_read
-	.quad sys_write
+	.quad compat_sys_read
+	.quad compat_sys_write
 	.quad sys32_open		/* 5 */
 	.quad sys_close
 	.quad sys32_waitpid

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

[PATCH, for review 3/3] fix evdev

[Stephen Rothwell]

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

This is drivers/input/evdev.c fixed to use the new infrastructure and the existing compat bitmap copying routine.
---
 evdev.c |   54 ++++++++++++------------------------------------------
 1 files changed, 12 insertions(+), 42 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN linus-compat_write.4/drivers/input/evdev.c linus-compat_write.5/drivers/input/evdev.c
--- linus-compat_write.4/drivers/input/evdev.c	2005-06-28 10:05:26.000000000 +1000
+++ linus-compat_write.5/drivers/input/evdev.c	2005-07-01 00:51:37.000000000 +1000
@@ -154,16 +154,6 @@
 	__s32 value;
 };
 
-#ifdef CONFIG_X86_64
-#  define COMPAT_TEST test_thread_flag(TIF_IA32)
-#elif defined(CONFIG_IA64)
-#  define COMPAT_TEST IS_IA32_PROCESS(ia64_task_regs(current))
-#elif defined(CONFIG_ARCH_S390)
-#  define COMPAT_TEST test_thread_flag(TIF_31BIT)
-#else
-#  define COMPAT_TEST test_thread_flag(TIF_32BIT)
-#endif
-
 static ssize_t evdev_write_compat(struct file * file, const char __user * buffer, size_t count, loff_t *ppos)
 {
 	struct evdev_list *list = file->private_data;
@@ -179,6 +169,8 @@
 
 	return retval;
 }
+#else
+#define evdev_write_compat	NULL
 #endif
 
 static ssize_t evdev_write(struct file * file, const char __user * buffer, size_t count, loff_t *ppos)
@@ -189,11 +181,6 @@
 
 	if (!list->evdev->exist) return -ENODEV;
 
-#ifdef CONFIG_COMPAT
-	if (COMPAT_TEST)
-		return evdev_write_compat(file, buffer, count, ppos);
-#endif
-
 	while (retval < count) {
 
 		if (copy_from_user(&event, buffer + retval, sizeof(struct input_event)))
@@ -243,6 +230,8 @@
 
 	return retval;
 }
+#else
+#define evdev_read_compat	NULL
 #endif
 
 static ssize_t evdev_read(struct file * file, char __user * buffer, size_t count, loff_t *ppos)
@@ -250,11 +239,6 @@
 	struct evdev_list *list = file->private_data;
 	int retval;
 
-#ifdef CONFIG_COMPAT
-	if (COMPAT_TEST)
-		return evdev_read_compat(file, buffer, count, ppos);
-#endif
-
 	if (count < sizeof(struct input_event))
 		return -EINVAL;
 
@@ -484,34 +468,18 @@
 
 #ifdef CONFIG_COMPAT
 
-#define BITS_PER_LONG_COMPAT (sizeof(compat_long_t) * 8)
-#define NBITS_COMPAT(x) ((((x)-1)/BITS_PER_LONG_COMPAT)+1)
-#define OFF_COMPAT(x)  ((x)%BITS_PER_LONG_COMPAT)
+#define NBITS_COMPAT(x) ((((x)-1)/BITS_PER_COMPAT_LONG)+1)
+#define OFF_COMPAT(x)  ((x)%BITS_PER_COMPAT_LONG)
 #define BIT_COMPAT(x)  (1UL<<OFF_COMPAT(x))
-#define LONG_COMPAT(x) ((x)/BITS_PER_LONG_COMPAT)
+#define LONG_COMPAT(x) ((x)/BITS_PER_COMPAT_LONG)
 #define test_bit_compat(bit, array) ((array[LONG_COMPAT(bit)] >> OFF_COMPAT(bit)) & 1)
 
-#ifdef __BIG_ENDIAN
 #define bit_to_user(bit, max) \
 do { \
-	int i; \
 	int len = NBITS_COMPAT((max)) * sizeof(compat_long_t); \
 	if (len > _IOC_SIZE(cmd)) len = _IOC_SIZE(cmd); \
-	for (i = 0; i < len / sizeof(compat_long_t); i++) \
-		if (copy_to_user((compat_long_t*) p + i, \
-				 (compat_long_t*) (bit) + i + 1 - ((i % 2) << 1), \
-				 sizeof(compat_long_t))) \
-			return -EFAULT; \
-	return len; \
+	return compat_put_bitmap(p, (bit), len * 8) ? -EFAULT : len; \
 } while (0)
-#else
-#define bit_to_user(bit, max) \
-do { \
-	int len = NBITS_COMPAT((max)) * sizeof(compat_long_t); \
-	if (len > _IOC_SIZE(cmd)) len = _IOC_SIZE(cmd); \
-	return copy_to_user(p, (bit), len) ? -EFAULT : len; \
-} while (0)
-#endif
 
 static long evdev_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -631,19 +599,21 @@
 	}
 	return -EINVAL;
 }
+#else
+#define evdev_ioctl_compat	NULL
 #endif
 
 static struct file_operations evdev_fops = {
 	.owner =	THIS_MODULE,
 	.read =		evdev_read,
+	.compat_read =	evdev_read_compat,
 	.write =	evdev_write,
+	.compat_write =	evdev_write_compat,
 	.poll =		evdev_poll,
 	.open =		evdev_open,
 	.release =	evdev_release,
 	.unlocked_ioctl = evdev_ioctl,
-#ifdef CONFIG_COMPAT
 	.compat_ioctl =	evdev_ioctl_compat,
-#endif
 	.fasync =	evdev_fasync,
 	.flush =	evdev_flush
 };

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

Re: [PATCH, for review 1/3] compat_sys_{read,write}

[David Howells]

Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> This patch introduces compatibility syscalls for read and write and the
> equivalent hooks in struct file_operations.  These are neede to support
> the badly designed (i.e. broken) input layer evdev read and write
> interfaces.

Should this also extend to the pread/pwrite syscalls?

> +asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf,
> +		size_t count)
> +{
> +	return do_sys_write(fd, buf, count, 0);
> +}

I think my biggest objection to this is that it'll stick another "frame" on
the stack. For something like this the compiler probably can't tail-call, at
least not on archs like i386 where the extra argument is passed on the stack.
For an arch that passes at least the first four arguments in registers, it
probably can, but I don't know that it will. Have you waved this in front of
any compiler hackers?

David

Re: [PATCH, for review 1/3] compat_sys_{read,write}

[Stephen Rothwell]

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

On Fri, 01 Jul 2005 08:58:25 +0100 David Howells <dhowells@redhat.com> wrote:
>
> Should this also extend to the pread/pwrite syscalls?

maybe ... :-)

> I think my biggest objection to this is that it'll stick another "frame" on
> the stack. For something like this the compiler probably can't tail-call, at
> least not on archs like i386 where the extra argument is passed on the stack.
> For an arch that passes at least the first four arguments in registers, it
> probably can, but I don't know that it will. Have you waved this in front of
> any compiler hackers?

On i386, it doesn't do a tail call as you said.  On PPC64, it takes two
instructions to produce the tail call ...  We could inline
do_sys_{read,write} if it was a real worry.  It should only add 32 bytes
(or less) to the stack on i386, shouldn't it?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH, for review 1/3] compat_sys_{read,write}

[Stephen Rothwell]

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

On Fri, 1 Jul 2005 14:56:42 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> This patch introduces compatibility syscalls for read and write and the
> equivalent hooks in struct file_operations.  These are neede to support
> the badly designed (i.e. broken) input layer evdev read and write
> interfaces.

Have you all come to some consensus on this issue?  Do I submit the
patches for compat_sys_read/write to Linus/Andrew?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH, for review 1/3] compat_sys_{read,write}

[David S. Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [PATCH, for review 1/3] compat_sys_{read,write}
Date: Thu, 21 Jul 2005 17:05:52 +1000

> On Fri, 1 Jul 2005 14:56:42 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > This patch introduces compatibility syscalls for read and write and the
> > equivalent hooks in struct file_operations.  These are neede to support
> > the badly designed (i.e. broken) input layer evdev read and write
> > interfaces.
> 
> Have you all come to some consensus on this issue?  Do I submit the
> patches for compat_sys_read/write to Linus/Andrew?

Most people are likely in Ottawa and don't have the time to
review your changes at the moment.

Re: [PATCH, for review 1/3] compat_sys_{read,write}

[Stephen Rothwell]

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

On Thu, 21 Jul 2005 01:05:13 -0700 (PDT) "David S. Miller" <davem@davemloft.net> wrote:
>
> Most people are likely in Ottawa and don't have the time to
> review your changes at the moment.

I know that, but we had this discussion almost three weeks ago and there was no final
resolution AND we wanted something for 2.6.13, so I was looking for something to do
and the patches would be waiting for Andrew and Linus when they return ...

(and, of course, since you are almost all there together, maybe you could discuss it over
dinner or something and let me know how much time I should waste on this ... :-))

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: RFC: is_compat_task

[Andi Kleen]

> Why don't we do the following to resolve this issue?
> Create a "is_compat_syscall()" macro, in compat syscall
> trap entry, the bit is set, and it is cleared otherwise.

It would work for me, however I think it would be cleaner
to add compat_read/write for input.

-Andi

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Stephen Rothwell]

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

On Tue, 28 Jun 2005 11:17:04 +0200 Andi Kleen <ak@suse.de> wrote:
>
> On Tue, Jun 28, 2005 at 06:14:53PM +1000, Stephen Rothwell wrote:
> > On Tue, 28 Jun 2005 09:27:10 +0200 Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > besides this beeing butt ugly didn't we have a rule about never checking
> > > task flags whether we're a compat process?  And if we're going to allow
> > > it it should be done properly.
> > 
> > So, I have been thinking about this patch for a while (wondering how badly
> > flamed I would get when I posted it :-)).  What do you all think?  Is this
> > a reasonable thing to do?
> 
> No, it's not because it makes it impossible to have 64bit processes
> that run with 32bit ABI (not implemented right now but we don't
> want to break that).

Why does this make it impossible?  All I am defining is *arch specific*
macros that tell us if the kernel was entered through a compat syscall. If
your arch needs something other than my *examples*, then you are free to
define it.

> Please don't do this. It is absolutely the wrong thing to do.

Then please tell us what the right thing to do is.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Andi Kleen]

On Wed, Jun 29, 2005 at 02:09:44AM +1000, Stephen Rothwell wrote:
> On Tue, 28 Jun 2005 11:17:04 +0200 Andi Kleen <ak@suse.de> wrote:
> >
> > On Tue, Jun 28, 2005 at 06:14:53PM +1000, Stephen Rothwell wrote:
> > > On Tue, 28 Jun 2005 09:27:10 +0200 Christoph Hellwig <hch@lst.de> wrote:
> > > >
> > > > besides this beeing butt ugly didn't we have a rule about never checking
> > > > task flags whether we're a compat process?  And if we're going to allow
> > > > it it should be done properly.
> > > 
> > > So, I have been thinking about this patch for a while (wondering how badly
> > > flamed I would get when I posted it :-)).  What do you all think?  Is this
> > > a reasonable thing to do?
> > 
> > No, it's not because it makes it impossible to have 64bit processes
> > that run with 32bit ABI (not implemented right now but we don't
> > want to break that).
> 
> Why does this make it impossible?  All I am defining is *arch specific*
> macros that tell us if the kernel was entered through a compat syscall. If

No, you're defining an arch specific macro to tell you that
the current process was loaded from a 32bit executable.
It does not tell you anything how the kernel was entered.

Subtle but important difference.

> 
> > Please don't do this. It is absolutely the wrong thing to do.
> 
> Then please tell us what the right thing to do is.

The right thing to do is what all the other non broken compat
functions do - figure out from the original system call entry
point if you're in compat context or not.

This typically requires a special path.

-Andi

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Stephen Rothwell]

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

On Tue, 28 Jun 2005 18:14:14 +0200 Andi Kleen <ak@suse.de> wrote:
>
> No, you're defining an arch specific macro to tell you that
> the current process was loaded from a 32bit executable.
> It does not tell you anything how the kernel was entered.
> 
> Subtle but important difference.

Thats why my patch was just an example implementation (that happens to
work for us currently).

> The right thing to do is what all the other non broken compat
> functions do - figure out from the original system call entry
> point if you're in compat context or not.
> 
> This typically requires a special path.

At least up to the point that you can flag it for further processing (see
the socket control message processing code).

We still have the signal delivery problem - a 32 bit binary running on
ppc64 needs the correct signal stack etc set up for it.  I guess most of
the 64bit arches have the equivalent of

        if (test_thread_flag(TIF_32BIT))
                return do_signal32(oldset, regs);

in do_signal().

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Andi Kleen]

> We still have the signal delivery problem - a 32 bit binary running on
> ppc64 needs the correct signal stack etc set up for it.  I guess most of
> the 64bit arches have the equivalent of
> 
>         if (test_thread_flag(TIF_32BIT))
>                 return do_signal32(oldset, regs);
> 
> in do_signal().

It's not a real problem. There are two use cases for this:
- A special 32bit in 64bit mode ABI. In this case the signal structures
can just be defined to have the necessary padding.
- A emulator like valgrind that executes a 32bit guest - in this
case the monitor can intercept the signal and handle it.
In both cases 64bit signals are fine.

-Andi

Re: RFC: is_compat_task (Was: Re: input compat stuff)

[Ralf Baechle]

On Tue, Jun 28, 2005 at 11:17:04AM +0200, Andi Kleen wrote:

> No, it's not because it makes it impossible to have 64bit processes
> that run with 32bit ABI (not implemented right now but we don't
> want to break that).

We do something like that on MIPS with N32.

(sigh...)

  Ralf

Re: input compat stuff

[Andi Kleen]

On Tue, Jun 28, 2005 at 09:27:10AM +0200, Christoph Hellwig wrote:
> http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=52658bb685df77f71e97f1b503dee97d27a88b0f;hp=024ac44c701d43f5e2d34bd6a35b2813a36e6010
> 
> besides this beeing butt ugly didn't we have a rule about never checking
> task flags whether we're a compat process?  And if we're going to allow
> it it should be done properly.

Yes, this is extremly bad and shouldn't be done. Can that
patch be reverted please?

-Andi