Re: posix message queues

[Arnd Bergmann <arnd@arndb.de>]

On Saturday 10 April 2004 13:53, Manfred Spraul wrote:
> Manfred Spraul wrote:
> 
> > 
> >+if (notification.sigev_notify == SIGEV_THREAD) {
> >+      if (copy_from_user(cookie, u_notification.sigev_value.sival_ptr,
> >
> notification, not u_notification: sival_ptr is a union on sival_int, and 
> sival_int was copied to kernel space by get_compat_sigevent.
> Updated patch attached, sorry.

I'm afraid that is still incorrect,  at least on big-endian machines:

> @@ -147,6 +148,14 @@
>         if (get_compat_sigevent(&notification, u_notification))
>                 return -EFAULT;
>  
> +       if (notification.sigev_notify == SIGEV_THREAD) {
> +               if (copy_from_user(cookie, notification.sigev_value.sival_ptr,
> +                                       NOTIFY_COOKIE_LEN)) {
> +                       return -EFAULT;
> +               }
> +               notification.sigev_value.sival_ptr = cookie;
> +       }

sival_ptr is not valid when passed through get_compat_sigevent, because
the 32 bit pointer might be copied to the upper half of the kernel pointer.
Even worse, the infamous s390 31 bit pointer conversion is missing.
This patch fixes that problem and two others reported by Jakub.

> > Arndt did 90% of the coding, it's already in 2.6.5-mm3.
BTW, my name is 'Arnd', not 'Arndt'. I hope we can stop this now before
this becomes some 'Russel' vs. 'Russell' problem ;-)

	Arnd <><

--
[PATCH] More fixups for compat_mq
- handle SIGEV_THREAD
- don't try to convert u_attr in sys_mq_open if !O_CREAT 
- handle __SI_MESGQ in copy_siginfo_to_user32  (still missing for
  ppc64 and parisc)
===== ipc/compat_mq.c 1.1 vs edited =====
--- 1.1/ipc/compat_mq.c	Sat Apr 10 16:46:12 2004
+++ edited/ipc/compat_mq.c	Sat Apr 10 20:48:02 2004
@@ -55,7 +55,7 @@
 	char *name;
 	long ret;
 
-	if (!u_attr)
+	if ((oflag & O_CREAT) == 0 || !u_attr)
 		return sys_mq_open(u_name, oflag, mode, 0);
 
 	if (get_compat_mq_attr(&attr, u_attr))
@@ -139,6 +139,8 @@
 {
 	mm_segment_t oldfs;
 	struct sigevent notification;
+	char cookie[NOTIFY_COOKIE_LEN];
+	compat_uptr_t u_cookie;
 	long ret;
 
 	if (!u_notification)
@@ -146,6 +148,15 @@
 
 	if (get_compat_sigevent(&notification, u_notification))
 		return -EFAULT;
+
+	if (notification.sigev_notify == SIGEV_THREAD) {
+		u_cookie = (compat_uptr_t)notification.sigev_value.sival_int;
+		if (copy_from_user(cookie, compat_ptr(u_cookie),
+						NOTIFY_COOKIE_LEN)) {
+			return -EFAULT;
+		}
+		notification.sigev_value.sival_ptr = cookie;
+	}
 
 	oldfs = get_fs();
 	set_fs(KERNEL_DS);
===== arch/ia64/ia32/ia32_signal.c 1.24 vs edited =====
--- 1.24/arch/ia64/ia32/ia32_signal.c	Wed Feb 25 11:31:13 2004
+++ edited/arch/ia64/ia32/ia32_signal.c	Sat Apr 10 21:06:07 2004
@@ -114,7 +114,12 @@
 			err |= __get_user(to->si_band, &from->si_band);
 			err |= __get_user(to->si_fd, &from->si_fd);
 			break;
-			/* case __SI_RT: This is not generated by the kernel as of now.  */
+		      case __SI_RT: /* This is not generated by the kernel as of now.  */
+		      case __SI_MESGQ:
+			err |= __get_user(to->si_pid, &from->si_pid);
+			err |= __get_user(to->si_uid, &from->si_uid);
+			err |= __get_user(to->si_int, &from->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/mips/kernel/signal32.c 1.13 vs edited =====
--- 1.13/arch/mips/kernel/signal32.c	Wed Feb 25 11:31:13 2004
+++ edited/arch/mips/kernel/signal32.c	Sat Apr 10 21:08:53 2004
@@ -358,7 +358,12 @@
 			err |= __put_user(from->si_band, &to->si_band);
 			err |= __put_user(from->si_fd, &to->si_fd);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_pid, &to->si_pid);
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/s390/kernel/compat_signal.c 1.7 vs edited =====
--- 1.7/arch/s390/kernel/compat_signal.c	Sat Mar 27 12:40:46 2004
+++ edited/arch/s390/kernel/compat_signal.c	Sat Apr 10 21:05:01 2004
@@ -74,6 +74,10 @@
 		err |= __copy_to_user(&to->_sifields._pad, &from->_sifields._pad, SI_PAD_SIZE);
 	else {
 		switch (from->si_code >> 16) {
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_int, &to->si_int);
+			/* fallthrough */
 		case __SI_KILL >> 16:
 			err |= __put_user(from->si_pid, &to->si_pid);
 			err |= __put_user(from->si_uid, &to->si_uid);
@@ -96,7 +100,6 @@
 			break;
 		default:
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
 		}
 	}
 	return err;
===== arch/sparc64/kernel/signal32.c 1.32 vs edited =====
--- 1.32/arch/sparc64/kernel/signal32.c	Fri Mar 26 23:16:00 2004
+++ edited/arch/sparc64/kernel/signal32.c	Sat Apr 10 21:13:56 2004
@@ -129,7 +129,12 @@
 			err |= __put_user(from->si_trapno, &to->si_trapno);
 			err |= __put_user((long)from->si_addr, &to->si_addr);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_pid, &to->si_pid);
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/x86_64/ia32/ia32_signal.c 1.19 vs edited =====
--- 1.19/arch/x86_64/ia32/ia32_signal.c	Mon Mar  8 15:23:47 2004
+++ edited/arch/x86_64/ia32/ia32_signal.c	Sat Apr 10 21:03:13 2004
@@ -85,7 +85,11 @@
 			err |= __put_user(from->si_overrun, &to->si_overrun); 
 			err |= __put_user((u32)(u64)from->si_ptr, &to->si_ptr);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;