[patch 18/50] add epoll compat code to kernel/compat.c

[patch 18/50] add epoll compat code to kernel/compat.c

[akpm]

From: Davide Libenzi <davidel@xmailserver.org>

Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are currently
using their own version of epoll compat_ code and they could probably wire to
the new common code.

Unfortunately, sys_epoll_pwait needs two compat versions, one for sigset_t
only, and one for sigset_t+epoll_event.

Architectures that do not require epoll_event translation [1] whould wire
compat_sys_epoll_pwait, while the ones that do require it, should wire
compat_sys_epoll_pwait2.

Architectures that do not require epoll_event translation, should *not* wire
neither compat_sys_epoll_ctl nor compat_sys_epoll_wait.  Patch over 2.6.20.


[1] An architecture needs epoll_event translation is alignof(u64) in 32 
    bits mode is 4, *and* alignof(u64) in 64 bits mode is 8.

[akpm@linux-foundation.org: cleanups]
Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: "Luck, Tony" <tony.luck@intel.com>
Cc: <linux-arch@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/eventpoll.c         |    9 +-
 include/linux/compat.h |   34 +++++++++
 kernel/compat.c        |  145 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 183 insertions(+), 5 deletions(-)

diff -puN fs/eventpoll.c~add-epoll-compat-code-to-kernel-compatc fs/eventpoll.c
--- a/fs/eventpoll.c~add-epoll-compat-code-to-kernel-compatc
+++ a/fs/eventpoll.c
@@ -544,8 +544,8 @@ eexit_1:
  * file descriptors inside the interest set.  It represents
  * the kernel part of the user space epoll_ctl(2).
  */
-asmlinkage long
-sys_epoll_ctl(int epfd, int op, int fd, struct epoll_event __user *event)
+asmlinkage long sys_epoll_ctl(int epfd, int op, int fd,
+				struct epoll_event __user *event)
 {
 	int error;
 	struct file *file, *tfile;
@@ -707,8 +707,9 @@ eexit_1:
  * part of the user space epoll_pwait(2).
  */
 asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events,
-		int maxevents, int timeout, const sigset_t __user *sigmask,
-		size_t sigsetsize)
+				int maxevents, int timeout,
+				const sigset_t __user *sigmask,
+				size_t sigsetsize)
 {
 	int error;
 	sigset_t ksigmask, sigsaved;
diff -puN include/linux/compat.h~add-epoll-compat-code-to-kernel-compatc include/linux/compat.h
--- a/include/linux/compat.h~add-epoll-compat-code-to-kernel-compatc
+++ a/include/linux/compat.h
@@ -234,5 +234,39 @@ asmlinkage long compat_sys_migrate_pages
 		compat_ulong_t maxnode, const compat_ulong_t __user *old_nodes,
 		const compat_ulong_t __user *new_nodes);
 
+/*
+ * epoll (fs/eventpoll.c) compat bits follow ...
+ */
+struct epoll_event;
+
+struct compat_epoll_event {
+	u32 events;
+	u32 data[2];
+};
+
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+				struct compat_epoll_event __user *event);
+asmlinkage long compat_sys_epoll_wait(int epfd,
+				struct compat_epoll_event __user *events,
+				int maxevents, int timeout);
+
+/*
+ * Architectures that does not need "struct epoll_event" translation
+ * should wire compat_sys_epoll_pwait. The ones that needs "struct epoll_event"
+ * translation, should wire compat_sys_epoll_pwait2. An architecture needs
+ * "struct epoll_event" translation is alignof(u64) in 32 bits mode is 4, and
+ * alignof(u64) in 64 bits mode is 8.
+ */
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+				struct epoll_event __user *events,
+				int maxevents, int timeout,
+				const compat_sigset_t __user *sigmask,
+				compat_size_t sigsetsize);
+asmlinkage long compat_sys_epoll_pwait2(int epfd,
+				struct compat_epoll_event __user *events,
+				int maxevents, int timeout,
+				const compat_sigset_t __user *sigmask,
+				compat_size_t sigsetsize);
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
diff -puN kernel/compat.c~add-epoll-compat-code-to-kernel-compatc kernel/compat.c
--- a/kernel/compat.c~add-epoll-compat-code-to-kernel-compatc
+++ a/kernel/compat.c
@@ -23,6 +23,7 @@
 #include <linux/timex.h>
 #include <linux/migrate.h>
 #include <linux/posix-timers.h>
+#include <linux/eventpoll.h>
 
 #include <asm/uaccess.h>
 
@@ -1017,6 +1018,149 @@ asmlinkage long compat_sys_migrate_pages
 }
 #endif
 
+#ifdef CONFIG_EPOLL
+
+/*
+ * epoll (fs/eventpoll.c) compat functions follow ...
+ */
+
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+				     struct compat_epoll_event __user *event)
+{
+	long err = 0;
+	struct compat_epoll_event user;
+	struct epoll_event __user *kernel = NULL;
+	union {
+		u64 q;
+		u32 d[2];
+	} mux;
+
+	if (event) {
+		if (copy_from_user(&user, event, sizeof(user)))
+			return -EFAULT;
+		kernel = compat_alloc_user_space(sizeof(struct epoll_event));
+		err |= __put_user(user.events, &kernel->events);
+		mux.d[0] = user.data[0];
+		mux.d[1] = user.data[1];
+		err |= __put_user(mux.q, &kernel->data);
+	}
+
+	return err ? err: sys_epoll_ctl(epfd, op, fd, kernel);
+}
+
+
+asmlinkage long compat_sys_epoll_wait(int epfd,
+				struct compat_epoll_event __user *events,
+				int maxevents, int timeout)
+{
+	long i, ret, err = 0;
+	struct epoll_event __user *kbuf;
+	struct epoll_event ev;
+	union {
+		u64 q;
+		u32 d[2];
+	} mux;
+
+		if (maxevents <= 0 || maxevents > (INT_MAX /
+						sizeof(struct epoll_event)))
+		return -EINVAL;
+		kbuf = compat_alloc_user_space(sizeof(struct epoll_event) *
+						maxevents);
+	ret = sys_epoll_wait(epfd, kbuf, maxevents, timeout);
+	for (i = 0; i < ret; i++) {
+		err |= __get_user(ev.events, &kbuf[i].events);
+		err |= __get_user(ev.data, &kbuf[i].data);
+		err |= put_user(ev.events, &events->events);
+		mux.q = ev.data;
+		err |= put_user(mux.d[0], &events->data[0]);
+		err |= put_user(mux.d[1], &events->data[1]);
+		events++;
+	}
+
+	return err ? -EFAULT: ret;
+}
+
+
+#ifdef TIF_RESTORE_SIGMASK
+
+static long __sys_epoll_pwait(int epfd, void __user *events, int maxevents,
+			int timeout, const compat_sigset_t __user *sigmask,
+			compat_size_t sigsetsize,
+			long (asmlinkage *proc)(int, void __user *, int, int))
+{
+	long err;
+	compat_sigset_t ss32;
+	sigset_t ksigmask, sigsaved;
+
+	/*
+	 * If the caller wants a certain signal mask to be set during the wait,
+	 * we apply it here.
+	 */
+	if (sigmask) {
+		if (sigsetsize != sizeof(compat_sigset_t))
+			return -EINVAL;
+		if (copy_from_user(&ss32, sigmask, sizeof(ss32)))
+			return -EFAULT;
+		sigset_from_compat(&ksigmask, &ss32);
+		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
+	}
+
+	err = (*proc)(epfd, events, maxevents, timeout);
+
+	/*
+	 * If we changed the signal mask, we need to restore the original one.
+	 * In case we've got a signal while waiting, we do not restore the
+	 * signal mask yet, and we allow do_signal() to deliver the signal on
+	 * the way back to userspace, before the signal mask is restored.
+	 */
+	if (sigmask) {
+		if (err == -EINTR) {
+			memcpy(&current->saved_sigmask, &sigsaved,
+			       sizeof(sigsaved));
+			set_thread_flag(TIF_RESTORE_SIGMASK);
+		} else
+			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
+	}
+
+	return err;
+}
+
+/*
+ * Architectures that does not need "struct epoll_event" translation
+ * should wire compat_sys_epoll_pwait. The ones that needs "struct epoll_event"
+ * translation, should wire compat_sys_epoll_pwait2. An architecture needs
+ * "struct epoll_event" translation is alignof(u64) in 32 bits mode is 4, and
+ * alignof(u64) in 64 bits mode is 8.
+ */
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+				struct epoll_event __user *events,
+				int maxevents, int timeout,
+				const compat_sigset_t __user *sigmask,
+				compat_size_t sigsetsize)
+{
+	return __sys_epoll_pwait(epfd, events, maxevents, timeout, sigmask,
+			sigsetsize,
+			(long (asmlinkage *)(int, void __user *, int, int))
+				sys_epoll_wait);
+}
+
+
+asmlinkage long compat_sys_epoll_pwait2(int epfd,
+				struct compat_epoll_event __user *events,
+				int maxevents, int timeout,
+				const compat_sigset_t __user *sigmask,
+				compat_size_t sigsetsize)
+{
+	return __sys_epoll_pwait(epfd, events, maxevents, timeout, sigmask,
+			sigsetsize,
+			(long (asmlinkage *)(int, void __user *, int, int))
+				compat_sys_epoll_wait);
+}
+
+#endif /* TIF_RESTORE_SIGMASK */
+#endif /* CONFIG_EPOLL */
+
 struct compat_sysinfo {
 	s32 uptime;
 	u32 loads[3];
@@ -1081,4 +1225,3 @@ compat_sys_sysinfo(struct compat_sysinfo
 
 	return 0;
 }
-
_

Re: [patch 18/50] add epoll compat code to kernel/compat.c

[Stephen Rothwell]

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

On Tue, 20 Feb 2007 13:57:58 -0800 akpm@linux-foundation.org wrote:
>
> From: Davide Libenzi <davidel@xmailserver.org>
>
> Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are currently
> using their own version of epoll compat_ code and they could probably wire to
> the new common code.

So did you just ignore the discussion that went on about this patch and
the suggestions made (by me) and accepted by the author?  i.e. why has
this gone to Linus in this form?

Nacked-by: Stephen Rothwell <sfr@canb.auug.org.au>

Whatever that means.
--
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 18/50] add epoll compat code to kernel/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, Stephen Rothwell wrote:

> On Tue, 20 Feb 2007 13:57:58 -0800 akpm@linux-foundation.org wrote:
> >
> > From: Davide Libenzi <davidel@xmailserver.org>
> >
> > Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are currently
> > using their own version of epoll compat_ code and they could probably wire to
> > the new common code.
> 
> So did you just ignore the discussion that went on about this patch and
> the suggestions made (by me) and accepted by the author?  i.e. why has
> this gone to Linus in this form?

Reminder for Andrew ...
We talked about having an asm/compat.h define the proper 
compat_epoll_event structure, and having a define in there that triggers 
the proper code inside kernel/compat.c. It looks like the better way to 
go, as Stephen suggested.
If that's accepted by everyone, I can go ahead and make a patch. Lemme 
know ...



- Davide

Re: [patch 18/50] add epoll compat code to kernel/compat.c

[Andrew Morton]

On Wed, 21 Feb 2007 10:20:27 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> On Tue, 20 Feb 2007 13:57:58 -0800 akpm@linux-foundation.org wrote:
> >
> > From: Davide Libenzi <davidel@xmailserver.org>
> >
> > Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are currently
> > using their own version of epoll compat_ code and they could probably wire to
> > the new common code.
> 
> So did you just ignore the discussion that went on about this patch and
> the suggestions made (by me) and accepted by the author?

That was a week ago and it's all gone quiet, so I guess I assumed we're good to go.

> Nacked-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> Whatever that means.

OK, I'll drop it.

Re: [patch 18/50] add epoll compat code to kernel/compat.c

[Stephen Rothwell]

Hi Davide,

> Reminder for Andrew ...
> We talked about having an asm/compat.h define the proper 
> compat_epoll_event structure, and having a define in there that triggers 
> the proper code inside kernel/compat.c. It looks like the better way to 
> go, as Stephen suggested.
> If that's accepted by everyone, I can go ahead and make a patch. Lemme 
> know ...

Like below?  I did the above and moved the code to fs/compat.c (from
kernel/compat.c).  Also MIPS already has a compat_sys_epoll_pwait, so I
had to add an #ifndef CONFIG_MIPS.

Compile tested on PowerPC.  No architectures are wired up yet.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Davide Libenzi <davidel@xmailserver.org>

Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are
currently using their own version of epoll compat_ code.

An architecture needs epoll_event translation if alignof(u64) in 32 bit
mode is different from alignof(u64) in 64 bit mode.  If an architecture
needs epoll_event translation, it must define struct compat_epoll_event
in asm/compat.h and set CONFIG_HAVE_COMPAT_EPOLL_EVENT and use
compat_sys_epoll_ctl and compat_sys_epoll_wait.

All 64 bit architecture should use compat_sys_epoll_pwait.

[sfr: restructure and move to fs/compat.c]

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/compat.c            |  102 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/compat.h |   20 +++++++++
 2 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index 0ec70e3..ace84d5 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -48,6 +48,7 @@
 #include <linux/highmem.h>
 #include <linux/poll.h>
 #include <linux/mm.h>
+#include <linux/eventpoll.h>
 
 #include <net/sock.h>		/* siocdevprivate_ioctl */
 
@@ -2235,3 +2236,104 @@ long asmlinkage compat_sys_nfsservctl(int cmd, void *notused, void *notused2)
 	return sys_ni_syscall();
 }
 #endif
+
+#ifdef CONFIG_EPOLL
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+			struct compat_epoll_event __user *event)
+{
+	long err = 0;
+	struct compat_epoll_event user;
+	struct epoll_event __user *kernel = NULL;
+
+	if (event) {
+		if (copy_from_user(&user, event, sizeof(user)))
+			return -EFAULT;
+		kernel = compat_alloc_user_space(sizeof(struct epoll_event));
+		err |= __put_user(user.events, &kernel->events);
+		err |= __put_user(user.data, &kernel->data);
+	}
+
+	return err ? err : sys_epoll_ctl(epfd, op, fd, kernel);
+}
+
+
+asmlinkage long compat_sys_epoll_wait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout)
+{
+	long i, ret, err = 0;
+	struct epoll_event __user *kbuf;
+	struct epoll_event ev;
+
+	if ((maxevents <= 0) ||
+			(maxevents > (INT_MAX / sizeof(struct epoll_event))))
+		return -EINVAL;
+	kbuf = compat_alloc_user_space(sizeof(struct epoll_event) * maxevents);
+	ret = sys_epoll_wait(epfd, kbuf, maxevents, timeout);
+	for (i = 0; i < ret; i++) {
+		err |= __get_user(ev.events, &kbuf[i].events);
+		err |= __get_user(ev.data, &kbuf[i].data);
+		err |= put_user(ev.events, &events->events);
+		err |= put_user(ev.data, &events->data);
+		events++;
+	}
+
+	return err ? -EFAULT: ret;
+}
+#endif	/* CONFIG_HAS_COMPAT_EPOLL_EVENT */
+
+#ifndef CONFIG_MIPS
+#ifdef TIF_RESTORE_SIGMASK
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout,
+			const compat_sigset_t __user *sigmask,
+			compat_size_t sigsetsize)
+{
+	long err;
+	compat_sigset_t csigmask;
+	sigset_t ksigmask, sigsaved;
+
+	/*
+	 * If the caller wants a certain signal mask to be set during the wait,
+	 * we apply it here.
+	 */
+	if (sigmask) {
+		if (sigsetsize != sizeof(compat_sigset_t))
+			return -EINVAL;
+		if (copy_from_user(&csigmask, sigmask, sizeof(csigmask)))
+			return -EFAULT;
+		sigset_from_compat(&ksigmask, &csigmask);
+		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
+	}
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+	err = compat_sys_epoll_wait(epfd, events, maxevents, timeout);
+#else
+	err = sys_epoll_wait(epfd, events, maxevents, timeout);
+#endif
+
+	/*
+	 * If we changed the signal mask, we need to restore the original one.
+	 * In case we've got a signal while waiting, we do not restore the
+	 * signal mask yet, and we allow do_signal() to deliver the signal on
+	 * the way back to userspace, before the signal mask is restored.
+	 */
+	if (sigmask) {
+		if (err == -EINTR) {
+			memcpy(&current->saved_sigmask, &sigsaved,
+			       sizeof(sigsaved));
+			set_thread_flag(TIF_RESTORE_SIGMASK);
+		} else
+			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
+	}
+
+	return err;
+}
+#endif /* TIF_RESTORE_SIGMASK */
+#endif /* !CONFIG_MIPS */
+
+#endif /* CONFIG_EPOLL */
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 80b17f4..de4ba81 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -234,5 +234,25 @@ asmlinkage long compat_sys_migrate_pages(compat_pid_t pid,
 		compat_ulong_t maxnode, const compat_ulong_t __user *old_nodes,
 		const compat_ulong_t __user *new_nodes);
 
+/*
+ * epoll (fs/eventpoll.c) compat bits follow ...
+ */
+struct epoll_event;
+
+#ifndef CONFIG_HAS_COMPAT_EPOLL_EVENT
+#define compat_epoll_event	epoll_event
+#else
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+			struct compat_epoll_event __user *event);
+asmlinkage long compat_sys_epoll_wait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout);
+#endif
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout,
+			const compat_sigset_t __user *sigmask,
+			compat_size_t sigsetsize);
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
-- 
1.4.4.4

Re: [patch 18/50] add epoll compat code to kernel/compat.c

[Stephen Rothwell]

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

On Wed, 21 Feb 2007 12:01:45 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Add epoll compat_ code to kernel/compat.c.  IA64 and ARM-OABI are
                            ^^^^^^^^^^^^^^^
Should be fs/compat.c, of course :-(

--
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 18/50] add epoll compat code to kernel/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, Stephen Rothwell wrote:

> Hi Davide,
> 
> > Reminder for Andrew ...
> > We talked about having an asm/compat.h define the proper 
> > compat_epoll_event structure, and having a define in there that triggers 
> > the proper code inside kernel/compat.c. It looks like the better way to 
> > go, as Stephen suggested.
> > If that's accepted by everyone, I can go ahead and make a patch. Lemme 
> > know ...
> 
> Like below?  I did the above and moved the code to fs/compat.c (from
> kernel/compat.c).  Also MIPS already has a compat_sys_epoll_pwait, so I
> had to add an #ifndef CONFIG_MIPS.

I was thinking something different. Example:

[include/asm-XXX/compat.h]

	struct compat_epoll_event {
		...
	} ATTR ...;
	#define NEED_EPOLL_EVENT_COMPAT 1


[kernel/compat.c]

	#ifdef CONFIG_EPOLL

	#ifdef NEED_EPOLL_EVENT_COMPAT
	....
	#endif

	#endif


That way we don't have arch-specific (MIPS, IA64, ...) ifdefs inside 
common code.




- Davide

Re: [patch 18/50] add epoll compat code to kernel/compat.c

[Ralf Baechle]

On Wed, Feb 21, 2007 at 12:01:45PM +1100, Stephen Rothwell wrote:

> > Reminder for Andrew ...
> > We talked about having an asm/compat.h define the proper 
> > compat_epoll_event structure, and having a define in there that triggers 
> > the proper code inside kernel/compat.c. It looks like the better way to 
> > go, as Stephen suggested.
> > If that's accepted by everyone, I can go ahead and make a patch. Lemme 
> > know ...
> 
> Like below?  I did the above and moved the code to fs/compat.c (from
> kernel/compat.c).  Also MIPS already has a compat_sys_epoll_pwait, so I
> had to add an #ifndef CONFIG_MIPS.

I suggest to remove the MIPS implementation from arch/mips/kernel/linux32.c
instead.  Aside of a microoptimization it's identical to the one you've
posted.

  Ralf

[PATCH]add epoll compat code to fsl/compat.c

[Stephen Rothwell]

From: Davide Libenzi <davidel@xmailserver.org>

Add epoll compat_ code to fs/compat.c.  IA64 and ARM-OABI are
currently using their own version of epoll compat_ code.

An architecture needs epoll_event translation if alignof(u64) in 32 bit
mode is different from alignof(u64) in 64 bit mode.  If an architecture
needs epoll_event translation, it must define struct compat_epoll_event
in asm/compat.h, set CONFIG_HAVE_COMPAT_EPOLL_EVENT and use
compat_sys_epoll_ctl and compat_sys_epoll_wait.  I suspect that the only
architecture that needs CONFIG_HAVE_COMPAT_EPOLL_EVENT is IA64.

All 64 bit architecture should use compat_sys_epoll_pwait.

[sfr: restructure and move to fs/compat.c, remove MIPS version
of compat_sys_epoll_pwait]

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/mips/kernel/linux32.c |   46 --------------------
 fs/compat.c                |  100 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/compat.h     |   19 ++++++++
 3 files changed, 119 insertions(+), 46 deletions(-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

On Tue, 20 Feb 2007 17:12:14 -0800 (PST) Davide Libenzi <davidel@xmailserver.org> wrote:
>
> I was thinking something different. Example:
> 
> [include/asm-XXX/compat.h]
> 
> 	struct compat_epoll_event {
> 		...
> 	} ATTR ...;
> 	#define NEED_EPOLL_EVENT_COMPAT 1

I am thinking the same except using CONFIG_HAS_COMPAT_EVENT_POLL
instead.  We seem to be putting stuff like this in Kconfig these days.

> That way we don't have arch-specific (MIPS, IA64, ...) ifdefs inside 
> common code.

The MIPS check was a temporary as I couldn't easily tell if the MIPS one was the same as the generic one.  Since Ralf has blessed it, this version of the patch removes the MIPS one and the checks for CONFIG_MIPS.

diff --git a/arch/mips/kernel/linux32.c b/arch/mips/kernel/linux32.c
index fc4dd6c..3432be5 100644
--- a/arch/mips/kernel/linux32.c
+++ b/arch/mips/kernel/linux32.c
@@ -737,49 +737,3 @@ _sys32_clone(nabi_no_regargs struct pt_regs regs)
 	return do_fork(clone_flags, newsp, &regs, 0,
 	               parent_tidptr, child_tidptr);
 }
-
-/*
- * Implement the event wait interface for the eventpoll file. It is the kernel
- * part of the user space epoll_pwait(2).
- */
-asmlinkage long compat_sys_epoll_pwait(int epfd,
-	struct epoll_event __user *events, int maxevents, int timeout,
-	const compat_sigset_t __user *sigmask, size_t sigsetsize)
-{
-	int error;
-	sigset_t ksigmask, sigsaved;
-
-	/*
-	 * If the caller wants a certain signal mask to be set during the wait,
-	 * we apply it here.
-	 */
-	if (sigmask) {
-		if (sigsetsize != sizeof(sigset_t))
-			return -EINVAL;
-		if (!access_ok(VERIFY_READ, sigmask, sizeof(ksigmask)))
-			return -EFAULT;
-		if (__copy_conv_sigset_from_user(&ksigmask, sigmask))
-			return -EFAULT;
-		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
-		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
-	}
-
-	error = sys_epoll_wait(epfd, events, maxevents, timeout);
-
-	/*
-	 * If we changed the signal mask, we need to restore the original one.
-	 * In case we've got a signal while waiting, we do not restore the
-	 * signal mask yet, and we allow do_signal() to deliver the signal on
-	 * the way back to userspace, before the signal mask is restored.
-	 */
-	if (sigmask) {
-		if (error == -EINTR) {
-			memcpy(&current->saved_sigmask, &sigsaved,
-				sizeof(sigsaved));
-			set_thread_flag(TIF_RESTORE_SIGMASK);
-		} else
-			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
-	}
-
-	return error;
-}
diff --git a/fs/compat.c b/fs/compat.c
index 0ec70e3..db1752f 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -48,6 +48,7 @@
 #include <linux/highmem.h>
 #include <linux/poll.h>
 #include <linux/mm.h>
+#include <linux/eventpoll.h>
 
 #include <net/sock.h>		/* siocdevprivate_ioctl */
 
@@ -2235,3 +2236,102 @@ long asmlinkage compat_sys_nfsservctl(int cmd, void *notused, void *notused2)
 	return sys_ni_syscall();
 }
 #endif
+
+#ifdef CONFIG_EPOLL
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+			struct compat_epoll_event __user *event)
+{
+	long err = 0;
+	struct compat_epoll_event user;
+	struct epoll_event __user *kernel = NULL;
+
+	if (event) {
+		if (copy_from_user(&user, event, sizeof(user)))
+			return -EFAULT;
+		kernel = compat_alloc_user_space(sizeof(struct epoll_event));
+		err |= __put_user(user.events, &kernel->events);
+		err |= __put_user(user.data, &kernel->data);
+	}
+
+	return err ? err : sys_epoll_ctl(epfd, op, fd, kernel);
+}
+
+
+asmlinkage long compat_sys_epoll_wait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout)
+{
+	long i, ret, err = 0;
+	struct epoll_event __user *kbuf;
+	struct epoll_event ev;
+
+	if ((maxevents <= 0) ||
+			(maxevents > (INT_MAX / sizeof(struct epoll_event))))
+		return -EINVAL;
+	kbuf = compat_alloc_user_space(sizeof(struct epoll_event) * maxevents);
+	ret = sys_epoll_wait(epfd, kbuf, maxevents, timeout);
+	for (i = 0; i < ret; i++) {
+		err |= __get_user(ev.events, &kbuf[i].events);
+		err |= __get_user(ev.data, &kbuf[i].data);
+		err |= put_user(ev.events, &events->events);
+		err |= put_user(ev.data, &events->data);
+		events++;
+	}
+
+	return err ? -EFAULT: ret;
+}
+#endif	/* CONFIG_HAS_COMPAT_EPOLL_EVENT */
+
+#ifdef TIF_RESTORE_SIGMASK
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout,
+			const compat_sigset_t __user *sigmask,
+			compat_size_t sigsetsize)
+{
+	long err;
+	compat_sigset_t csigmask;
+	sigset_t ksigmask, sigsaved;
+
+	/*
+	 * If the caller wants a certain signal mask to be set during the wait,
+	 * we apply it here.
+	 */
+	if (sigmask) {
+		if (sigsetsize != sizeof(compat_sigset_t))
+			return -EINVAL;
+		if (copy_from_user(&csigmask, sigmask, sizeof(csigmask)))
+			return -EFAULT;
+		sigset_from_compat(&ksigmask, &csigmask);
+		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
+	}
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+	err = compat_sys_epoll_wait(epfd, events, maxevents, timeout);
+#else
+	err = sys_epoll_wait(epfd, events, maxevents, timeout);
+#endif
+
+	/*
+	 * If we changed the signal mask, we need to restore the original one.
+	 * In case we've got a signal while waiting, we do not restore the
+	 * signal mask yet, and we allow do_signal() to deliver the signal on
+	 * the way back to userspace, before the signal mask is restored.
+	 */
+	if (sigmask) {
+		if (err == -EINTR) {
+			memcpy(&current->saved_sigmask, &sigsaved,
+			       sizeof(sigsaved));
+			set_thread_flag(TIF_RESTORE_SIGMASK);
+		} else
+			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
+	}
+
+	return err;
+}
+#endif /* TIF_RESTORE_SIGMASK */
+
+#endif /* CONFIG_EPOLL */
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 80b17f4..ccd863d 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -234,5 +234,24 @@ asmlinkage long compat_sys_migrate_pages(compat_pid_t pid,
 		compat_ulong_t maxnode, const compat_ulong_t __user *old_nodes,
 		const compat_ulong_t __user *new_nodes);
 
+/*
+ * epoll (fs/eventpoll.c) compat bits follow ...
+ */
+#ifndef CONFIG_HAS_COMPAT_EPOLL_EVENT
+struct epoll_event;
+#define compat_epoll_event	epoll_event
+#else
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+			struct compat_epoll_event __user *event);
+asmlinkage long compat_sys_epoll_wait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout);
+#endif
+asmlinkage long compat_sys_epoll_pwait(int epfd,
+			struct compat_epoll_event __user *events,
+			int maxevents, int timeout,
+			const compat_sigset_t __user *sigmask,
+			compat_size_t sigsetsize);
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
-- 
1.4.4.4

Re: [PATCH]add epoll compat code to fsl/compat.c

[Davide Libenzi]

Hi Stephen,

+asmlinkage long compat_sys_epoll_wait(int epfd,
+                       struct compat_epoll_event __user *events,
+                       int maxevents, int timeout)
+{
+       long i, ret, err = 0;
+       struct epoll_event __user *kbuf;
+       struct epoll_event ev;
+
+       if ((maxevents <= 0) ||
+                       (maxevents > (INT_MAX / sizeof(struct 
epoll_event))))
+               return -EINVAL;
+       kbuf = compat_alloc_user_space(sizeof(struct epoll_event) * 
maxevents);
+       ret = sys_epoll_wait(epfd, kbuf, maxevents, timeout);
+       for (i = 0; i < ret; i++) {
+               err |= __get_user(ev.events, &kbuf[i].events);
+               err |= __get_user(ev.data, &kbuf[i].data);
+               err |= put_user(ev.events, &events->events);
+               err |= put_user(ev.data, &events->data);
+               events++;
+       }
+
+       return err ? -EFAULT: ret;
+}


I don't think we can safely assume that we can access a potentially 
4-bytes aligned u64 on 64 bit architectures that require compat:

	put_user(ev.data, &events->data);

I thought again about the definition of "struct compat_epoll_event" and 
the only cases that we can have, are:

1) 8-bytes "data" member alignment on 32 bit and 64 bit (ie, SPARC)

   -> No need for compat

2) 4-bytes "data" member alignment on 32 bit and 64 bit (ie, X86-64 - 
   forced by "struct epoll_event" definition in eventpoll.h)

   -> No need for compat

3) 4-bytes "data" member alignment on 32 bit and 8-bytes on 64 bit
   (ie, IA-64)

   -> Need compat


So, the only case where we need compat, can be covered by the generic 
"struct compat_epoll_event" definition inside linux/compat.h. If we'd 
instead go for an asm/compat.h definition of "struct compat_epoll_event", 
then we'd probably need to define even special copy functions from/to 
"struct epoll_event" <-> "struct compat_epoll_event".
But that's not necessary IMO since the only case we can have is #3.
How does the patch below look to you (moved stuff to fs/compat.c and using 
CONFIG_HAS_COMPAT_EPOLL_EVENT)?




- Davide



diff -Nru linux-2.6.20/fs/compat.c linux-2.6.20.mod/fs/compat.c
--- linux-2.6.20/fs/compat.c	2007-02-21 11:47:00.000000000 -0800
+++ linux-2.6.20.mod/fs/compat.c	2007-02-21 12:04:26.000000000 -0800
@@ -48,6 +48,7 @@
 #include <linux/highmem.h>
 #include <linux/poll.h>
 #include <linux/mm.h>
+#include <linux/eventpoll.h>
 
 #include <net/sock.h>		/* siocdevprivate_ioctl */
 
@@ -2235,3 +2236,119 @@
 	return sys_ni_syscall();
 }
 #endif
+
+
+#ifdef CONFIG_EPOLL
+
+/*
+ * epoll (fs/eventpoll.c) compat functions follow ...
+ */
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+		struct compat_epoll_event __user *event)
+{
+	long err = 0;
+	struct compat_epoll_event user;
+	struct epoll_event __user *kernel = NULL;
+	union {
+		u64 q;
+		u32 d[2];
+	} mux;
+
+	if (event) {
+		if (copy_from_user(&user, event, sizeof(user)))
+			return -EFAULT;
+		kernel = compat_alloc_user_space(sizeof(struct epoll_event));
+		err |= __put_user(user.events, &kernel->events);
+		mux.d[0] = user.data[0];
+		mux.d[1] = user.data[1];
+		err |= __put_user(mux.q, &kernel->data);
+	}
+	
+	return err ? err: sys_epoll_ctl(epfd, op, fd, kernel);
+}
+
+asmlinkage long compat_sys_epoll_wait(int epfd, struct compat_epoll_event __user *events,
+		int maxevents, int timeout)
+{
+	long i, ret, err = 0;
+	struct epoll_event __user *kbuf;
+	struct epoll_event ev;
+	union {
+		u64 q;
+		u32 d[2];
+	} mux;
+
+	if (maxevents <= 0 || maxevents > (INT_MAX / sizeof(struct epoll_event)))
+		return -EINVAL;
+	kbuf = compat_alloc_user_space(sizeof(struct epoll_event) * maxevents);
+	ret = sys_epoll_wait(epfd, kbuf, maxevents, timeout);
+	for (i = 0; i < ret; i++) {
+		err |= __get_user(ev.events, &kbuf[i].events);
+		err |= __get_user(ev.data, &kbuf[i].data);
+		err |= put_user(ev.events, &events->events);
+		mux.q = ev.data;
+		err |= put_user(mux.d[0], &events->data[0]);
+		err |= put_user(mux.d[1], &events->data[1]);
+		events++;
+	}
+
+	return err ? -EFAULT: ret;	
+}
+
+#endif /* CONFIG_HAS_COMPAT_EPOLL_EVENT */
+
+#ifdef TIF_RESTORE_SIGMASK
+
+asmlinkage long compat_sys_epoll_pwait(int epfd, struct compat_epoll_event __user *events,
+		int maxevents, int timeout, const compat_sigset_t __user *sigmask,
+		compat_size_t sigsetsize)
+{
+	long err;
+	compat_sigset_t ss32;
+	sigset_t ksigmask, sigsaved;
+
+	/*
+	 * If the caller wants a certain signal mask to be set during the wait,
+	 * we apply it here.
+	 */
+	if (sigmask) {
+		if (sigsetsize != sizeof(compat_sigset_t))
+			return -EINVAL;
+		if (copy_from_user(&ss32, sigmask, sizeof(ss32)))
+			return -EFAULT;
+		sigset_from_compat(&ksigmask, &ss32);
+		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);		
+	}
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+	err = compat_sys_epoll_wait(epfd, events, maxevents, timeout);
+#else
+	err = sys_epoll_wait(epfd, events, maxevents, timeout);
+#endif
+
+	/*
+	 * If we changed the signal mask, we need to restore the original one.
+	 * In case we've got a signal while waiting, we do not restore the 
+	 * signal mask yet, and we allow do_signal() to deliver the signal on the way 
+	 * back to userspace, before the signal mask is restored.
+	 */
+	if (sigmask) {
+		if (err == -EINTR) {
+			memcpy(&current->saved_sigmask, &sigsaved, 
+			       sizeof(sigsaved));
+			set_thread_flag(TIF_RESTORE_SIGMASK);
+		} else
+			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
+	}
+
+	return err;	
+}
+
+#endif /* TIF_RESTORE_SIGMASK */
+
+#endif /* CONFIG_EPOLL */
+
diff -Nru linux-2.6.20/include/linux/compat.h linux-2.6.20.mod/include/linux/compat.h
--- linux-2.6.20/include/linux/compat.h	2007-02-09 16:14:20.000000000 -0800
+++ linux-2.6.20.mod/include/linux/compat.h	2007-02-21 12:06:04.000000000 -0800
@@ -234,5 +234,32 @@
 		compat_ulong_t maxnode, const compat_ulong_t __user *old_nodes,
 		const compat_ulong_t __user *new_nodes);
 
+/*
+ * epoll (fs/eventpoll.c) compat bits follow ...
+ */
+struct epoll_event;
+
+#ifdef CONFIG_HAS_COMPAT_EPOLL_EVENT
+
+struct compat_epoll_event {
+	u32 events;
+	u32 data[2];
+};
+
+asmlinkage long compat_sys_epoll_ctl(int epfd, int op, int fd,
+		struct compat_epoll_event __user *event);
+asmlinkage long compat_sys_epoll_wait(int epfd, struct compat_epoll_event __user *events,
+		int maxevents, int timeout);
+
+#else /* CONFIG_HAS_COMPAT_EPOLL_EVENT */
+
+#define compat_epoll_event epoll_event
+
+#endif /* CONFIG_HAS_COMPAT_EPOLL_EVENT */
+
+asmlinkage long compat_sys_epoll_pwait(int epfd, struct compat_epoll_event __user *events,
+		int maxevents, int timeout, const compat_sigset_t __user *sigmask,
+		compat_size_t sigsetsize);
+
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */

Re: [PATCH]add epoll compat code to fsl/compat.c

[Linus Torvalds]

On Wed, 21 Feb 2007, Davide Libenzi wrote:
> 
> I don't think we can safely assume that we can access a potentially 
> 4-bytes aligned u64 on 64 bit architectures that require compat:
> 
> 	put_user(ev.data, &events->data);

Oh, we can. "put_user()" and "get_user()" already have to work on totally 
unaligned data. If some architecture has problems with that, they have 
bigger issues, methinks.

We can't trust user pointers, and that includes not trusting them being 
aligned. 

		Linus

Re: [PATCH]add epoll compat code to fsl/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, Linus Torvalds wrote:

> 
> 
> On Wed, 21 Feb 2007, Davide Libenzi wrote:
> > 
> > I don't think we can safely assume that we can access a potentially 
> > 4-bytes aligned u64 on 64 bit architectures that require compat:
> > 
> > 	put_user(ev.data, &events->data);
> 
> Oh, we can. "put_user()" and "get_user()" already have to work on totally 
> unaligned data. If some architecture has problems with that, they have 
> bigger issues, methinks.
> 
> We can't trust user pointers, and that includes not trusting them being 
> aligned. 

Don't we get EFAULT in case of exception (access or alignment) in there?
For "dealing with", here we'd need them to do the correct thing 
(split-load?) in case of mis-aligned access.



- Davide

Re: [PATCH]add epoll compat code to fsl/compat.c

[Linus Torvalds]

On Wed, 21 Feb 2007, Davide Libenzi wrote:
> > 
> > We can't trust user pointers, and that includes not trusting them being 
> > aligned. 
> 
> Don't we get EFAULT in case of exception (access or alignment) in there?
> For "dealing with", here we'd need them to do the correct thing 
> (split-load?) in case of mis-aligned access.

I sure hope no architecture does that. EFAULT should be for *unmapped* 
accesses, not from users just using unaligned pointers. 

I would argue that it is a bug if some architecture thinks that 
"get_user()" and "put_user()" don't do unaligned accesses. 

(Not to say that it might not be something some odd architecture *does*, 
but I'd be disappointed if somebody did)

		Linus

Re: [PATCH]add epoll compat code to fsl/compat.c

[Ralf Baechle]

On Wed, Feb 21, 2007 at 01:04:51PM -0800, Davide Libenzi wrote:

> > Oh, we can. "put_user()" and "get_user()" already have to work on totally 
> > unaligned data. If some architecture has problems with that, they have 
> > bigger issues, methinks.
> > 
> > We can't trust user pointers, and that includes not trusting them being 
> > aligned. 
> 
> Don't we get EFAULT in case of exception (access or alignment) in there?
> For "dealing with", here we'd need them to do the correct thing 
> (split-load?) in case of mis-aligned access.

On an architecture which doesn't transparently handle missalignment the
best approach is to have the unalignment exception handler fix things
such that the 99.999% common case of properly aligned get_user/put_user
doesn't have to care.  A few nasty architectures (Afair old ARM and the
b0rked R5900 in the Playstation 2 for 128-bit integer loads) however
don't throw exceptions so __get_user / __put_user have to handle the
problem manually which of course adds considerable overhead to the common
case.

  Ralf

Re: [PATCH]add epoll compat code to fsl/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, Ralf Baechle wrote:

> On Wed, Feb 21, 2007 at 01:04:51PM -0800, Davide Libenzi wrote:
> 
> > > Oh, we can. "put_user()" and "get_user()" already have to work on totally 
> > > unaligned data. If some architecture has problems with that, they have 
> > > bigger issues, methinks.
> > > 
> > > We can't trust user pointers, and that includes not trusting them being 
> > > aligned. 
> > 
> > Don't we get EFAULT in case of exception (access or alignment) in there?
> > For "dealing with", here we'd need them to do the correct thing 
> > (split-load?) in case of mis-aligned access.
> 
> On an architecture which doesn't transparently handle missalignment the
> best approach is to have the unalignment exception handler fix things
> such that the 99.999% common case of properly aligned get_user/put_user
> doesn't have to care.  A few nasty architectures (Afair old ARM and the
> b0rked R5900 in the Playstation 2 for 128-bit integer loads) however
> don't throw exceptions so __get_user / __put_user have to handle the
> problem manually which of course adds considerable overhead to the common
> case.

Exactly. But at that point it'd better that the code handle the potential 
split operation by itself, w/out getting a fault for each mis-aligned 
access.



- Davide

Re: [PATCH]add epoll compat code to fsl/compat.c

[Ralf Baechle]

On Wed, Feb 21, 2007 at 02:25:09PM -0800, Davide Libenzi wrote:

> > doesn't have to care.  A few nasty architectures (Afair old ARM and the
> > b0rked R5900 in the Playstation 2 for 128-bit integer loads) however
> > don't throw exceptions so __get_user / __put_user have to handle the
> > problem manually which of course adds considerable overhead to the common
> > case.
> 
> Exactly. But at that point it'd better that the code handle the potential 
> split operation by itself, w/out getting a fault for each mis-aligned 
> access.

For these architectures it's a matter of correctness because ther won't
be an exception, so no chance to handle the rare case of missalignment
in the slow path.

  Ralf

Re: [PATCH]add epoll compat code to fsl/compat.c

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 21 Feb 2007 12:36:11 -0800 (PST)

> On Wed, 21 Feb 2007, Davide Libenzi wrote:
> > 
> > I don't think we can safely assume that we can access a potentially 
> > 4-bytes aligned u64 on 64 bit architectures that require compat:
> > 
> > 	put_user(ev.data, &events->data);
> 
> Oh, we can. "put_user()" and "get_user()" already have to work on totally 
> unaligned data. If some architecture has problems with that, they have 
> bigger issues, methinks.
> 
> We can't trust user pointers, and that includes not trusting them being 
> aligned. 

Right, and on Sparc for userland they are illegal so we'll just
SIGBUS the process if that happens.

Re: [PATCH]add epoll compat code to fsl/compat.c

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 21 Feb 2007 13:15:58 -0800 (PST)

> On Wed, 21 Feb 2007, Davide Libenzi wrote:
> > > 
> > > We can't trust user pointers, and that includes not trusting them being 
> > > aligned. 
> > 
> > Don't we get EFAULT in case of exception (access or alignment) in there?
> > For "dealing with", here we'd need them to do the correct thing 
> > (split-load?) in case of mis-aligned access.
> 
> I sure hope no architecture does that. EFAULT should be for *unmapped* 
> accesses, not from users just using unaligned pointers. 
> 
> I would argue that it is a bug if some architecture thinks that 
> "get_user()" and "put_user()" don't do unaligned accesses. 
> 
> (Not to say that it might not be something some odd architecture *does*, 
> but I'd be disappointed if somebody did)

If unaligned accesses in userspace give SIGBUS, which is the case
on both Sparc ports, it is a question of whether the same should be
done in cases like this.

I can definitely see arguments in both directions.

On the one hand, if userland has the have aligned stuff while he
executes, he should provide the same for things he gives to the
kernel.

On the other hand, userland has no way to embed structures passed into
system calls, using things like the "packed" GCC attribute, and
communicate that to the kernel.  So in that regard, the kernel should
try to handle the unaligned access as best as possible.

But currently we -EFAULT on sparc in this situation and I've never
seen a problem because of that.

Nevertheless, I am to understand that in this case we're talking about
a "u64" object, and that (and the structure it is in) will be aligned
on an 8-byte boundary on both sparc32 and sparc64, so maybe there is
no problem here?

Re: [PATCH]add epoll compat code to fsl/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, David Miller wrote:

> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed, 21 Feb 2007 12:36:11 -0800 (PST)
> 
> > On Wed, 21 Feb 2007, Davide Libenzi wrote:
> > > 
> > > I don't think we can safely assume that we can access a potentially 
> > > 4-bytes aligned u64 on 64 bit architectures that require compat:
> > > 
> > > 	put_user(ev.data, &events->data);
> > 
> > Oh, we can. "put_user()" and "get_user()" already have to work on totally 
> > unaligned data. If some architecture has problems with that, they have 
> > bigger issues, methinks.
> > 
> > We can't trust user pointers, and that includes not trusting them being 
> > aligned. 
> 
> Right, and on Sparc for userland they are illegal so we'll just
> SIGBUS the process if that happens.

Ahha! That's where I got fooled :) I looked at IA64 and I saw it had the 
proper unaligned case, but then I saw that SPARC was not handling the 
unaligned case.



- Davide

Re: [PATCH]add epoll compat code to fsl/compat.c

[Davide Libenzi]

On Wed, 21 Feb 2007, David Miller wrote:

> If unaligned accesses in userspace give SIGBUS, which is the case
> on both Sparc ports, it is a question of whether the same should be
> done in cases like this.
> 
> I can definitely see arguments in both directions.
> 
> On the one hand, if userland has the have aligned stuff while he
> executes, he should provide the same for things he gives to the
> kernel.
> 
> On the other hand, userland has no way to embed structures passed into
> system calls, using things like the "packed" GCC attribute, and
> communicate that to the kernel.  So in that regard, the kernel should
> try to handle the unaligned access as best as possible.
> 
> But currently we -EFAULT on sparc in this situation and I've never
> seen a problem because of that.
> 
> Nevertheless, I am to understand that in this case we're talking about
> a "u64" object, and that (and the structure it is in) will be aligned
> on an 8-byte boundary on both sparc32 and sparc64, so maybe there is
> no problem here?

SPARC is fine indeed, because it'll never wire the compat code for the 
"struct epoll_event" (because an u64 is 8 bytes aligned on both 32 and 
64 bit).
Mine was a conceptual issue, where for the only case that would the compat 
code for "struct epoll_event" cover (u64 align 4 bytes on 32 bit, and 8 
bytes on 64 bit), we are using {get,put}_user on an unaligned (4 bytes 
aligned) u64.
If all the architectures that require "struct epoll_event" compat, 
guarantee that a {get,put}_user of a u64 on unaligned memory works as 
expected, then Stephen code is fine.



- Davide

Re: [PATCH]add epoll compat code to fsl/compat.c

[Stephen Rothwell]

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

On Wed, 21 Feb 2007 19:07:02 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>
> Nevertheless, I am to understand that in this case we're talking about
> a "u64" object, and that (and the structure it is in) will be aligned
> on an 8-byte boundary on both sparc32 and sparc64, so maybe there is
> no problem here?

Yes, I am pretty sure that the only architecture that would need this is
IA64 where the u64 is 4 byte aligned in 32 bit mode and 8 byte aligned in
64 bit mode.  x86_64 declared the structure packed in 64 bit (so
presumably doesn't have a problem with unaligned u64 accesses).  All the
other 32 bit equivalent architectures align u64s on 8 byte boundaries (I
think).

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

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