linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Amanieu d'Antras" <amanieu@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Oleg Nesterov <oleg@redhat.com>,
	"Amanieu d'Antras" <amanieu@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org
Subject: [PATCH v2 04/20] x86: Rewrite copy_siginfo_{to,from}_user32
Date: Thu,  5 Nov 2015 00:50:23 +0000	[thread overview]
Message-ID: <1446684640-4112-5-git-send-email-amanieu@gmail.com> (raw)
In-Reply-To: <1446684640-4112-1-git-send-email-amanieu@gmail.com>

x86 can't use the generic versions because it needs to support
x32, so we replace the ad-hoc implementations with something
that is closer to the generic versions.

This is done by completely replacing the existing code with the
generic version, with some minor modifications to support x32.
The new code is kept as close as possible to the generic version
to make future changes to both functions easier.

Unlike the previous implementation, this one guarantees that the
compat behavior is identical to that of a 32-bit kernel.

Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
---
 arch/x86/kernel/signal_compat.c | 285 ++++++++++++++++++++++++++++++----------
 1 file changed, 214 insertions(+), 71 deletions(-)

diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index dc3c0b1..cdbb538 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -3,93 +3,236 @@
 
 int copy_siginfo_to_user32(compat_siginfo_t __user *to, const siginfo_t *from)
 {
-	int err = 0;
+	int err, si_code;
 	bool ia32 = test_thread_flag(TIF_IA32);
 
 	if (!access_ok(VERIFY_WRITE, to, sizeof(compat_siginfo_t)))
 		return -EFAULT;
 
-	put_user_try {
-		/* If you change siginfo_t structure, please make sure that
-		   this code is fixed accordingly.
-		   It should never copy any pad contained in the structure
-		   to avoid security leaks, but must copy the generic
-		   3 ints plus the relevant union member.  */
-		put_user_ex(from->si_signo, &to->si_signo);
-		put_user_ex(from->si_errno, &to->si_errno);
-		put_user_ex((short)from->si_code, &to->si_code);
+	/*
+	 * Get the user-visible si_code by hiding the top 16 bits if this is a
+	 * kernel-generated signal.
+	 */
+	si_code = from->si_code < 0 ? from->si_code : (short)from->si_code;
 
-		if (from->si_code < 0) {
-			put_user_ex(from->si_pid, &to->si_pid);
-			put_user_ex(from->si_uid, &to->si_uid);
-			put_user_ex(ptr_to_compat(from->si_ptr), &to->si_ptr);
+	/*
+	 * If you change siginfo_t structure, please be sure that
+	 * all these functions are fixed accordingly:
+	 * copy_siginfo_to_user
+	 * copy_siginfo_to_user32
+	 * copy_siginfo_from_user32
+	 * signalfd_copyinfo
+	 * They should never copy any pad contained in the structure
+	 * to avoid security leaks, but must copy the generic
+	 * 3 ints plus the relevant union member.
+	 */
+	err = __put_user(from->si_signo, &to->si_signo);
+	err |= __put_user(from->si_errno, &to->si_errno);
+	err |= __put_user(si_code, &to->si_code);
+	if (from->si_code < 0) {
+		/*
+		 * Copy the tail bytes of the union from the padding, see the
+		 * comment in copy_siginfo_from_user32. Note that this padding
+		 * is always initialized when si_code < 0.
+		 */
+		BUILD_BUG_ON(sizeof(to->_sifields._pad) !=
+			sizeof(from->_sifields._pad) + sizeof(from->_pad2));
+		err |= __copy_to_user(to->_sifields._pad, from->_sifields._pad,
+			sizeof(from->_sifields._pad)) ? -EFAULT : 0;
+		err |= __copy_to_user(to->_sifields._pad + SI_PAD_SIZE,
+			from->_pad2, sizeof(from->_pad2)) ? -EFAULT : 0;
+		return err;
+	}
+	switch (from->si_code & __SI_MASK) {
+	case __SI_KILL:
+		err |= __put_user(from->si_pid, &to->si_pid);
+		err |= __put_user(from->si_uid, &to->si_uid);
+		break;
+	case __SI_TIMER:
+		err |= __put_user(from->si_tid, &to->si_tid);
+		err |= __put_user(from->si_overrun, &to->si_overrun);
+		/*
+		 * Get the sigval from si_int, which matches the convention
+		 * used in get_compat_sigevent.
+		 */
+		err |= __put_user(from->si_int, &to->si_int);
+		break;
+	case __SI_POLL:
+		err |= __put_user(from->si_band, &to->si_band);
+		err |= __put_user(from->si_fd, &to->si_fd);
+		break;
+	case __SI_FAULT:
+		err |= __put_user(ptr_to_compat(from->si_addr), &to->si_addr);
+#ifdef __ARCH_SI_TRAPNO
+		err |= __put_user(from->si_trapno, &to->si_trapno);
+#endif
+#ifdef BUS_MCEERR_AO
+		/*
+		 * Other callers might not initialize the si_lsb field,
+		 * so check explicitly for the right codes here.
+		 */
+		if (from->si_signo == SIGBUS &&
+		    (from->si_code == BUS_MCEERR_AR ||
+		     from->si_code == BUS_MCEERR_AO))
+			err |= __put_user(from->si_addr_lsb, &to->si_addr_lsb);
+#endif
+#ifdef SEGV_BNDERR
+		if (from->si_signo == SIGSEGV && from->si_code == SEGV_BNDERR) {
+			err |= __put_user(ptr_to_compat(from->si_lower),
+				&to->si_lower);
+			err |= __put_user(ptr_to_compat(from->si_upper),
+				&to->si_upper);
+		}
+#endif
+		break;
+	case __SI_CHLD:
+		err |= __put_user(from->si_pid, &to->si_pid);
+		err |= __put_user(from->si_uid, &to->si_uid);
+		err |= __put_user(from->si_status, &to->si_status);
+		if (ia32) {
+			err |= __put_user(from->si_utime, &to->si_utime);
+			err |= __put_user(from->si_stime, &to->si_stime);
 		} else {
-			/*
-			 * First 32bits of unions are always present:
-			 * si_pid === si_band === si_tid === si_addr(LS half)
-			 */
-			put_user_ex(from->_sifields._pad[0],
-					  &to->_sifields._pad[0]);
-			switch (from->si_code >> 16) {
-			case __SI_FAULT >> 16:
-				break;
-			case __SI_SYS >> 16:
-				put_user_ex(from->si_syscall, &to->si_syscall);
-				put_user_ex(from->si_arch, &to->si_arch);
-				break;
-			case __SI_CHLD >> 16:
-				if (ia32) {
-					put_user_ex(from->si_utime, &to->si_utime);
-					put_user_ex(from->si_stime, &to->si_stime);
-				} else {
-					put_user_ex(from->si_utime, &to->_sifields._sigchld_x32._utime);
-					put_user_ex(from->si_stime, &to->_sifields._sigchld_x32._stime);
-				}
-				put_user_ex(from->si_status, &to->si_status);
-				/* FALL THROUGH */
-			default:
-			case __SI_KILL >> 16:
-				put_user_ex(from->si_uid, &to->si_uid);
-				break;
-			case __SI_POLL >> 16:
-				put_user_ex(from->si_fd, &to->si_fd);
-				break;
-			case __SI_TIMER >> 16:
-				put_user_ex(from->si_overrun, &to->si_overrun);
-				put_user_ex(ptr_to_compat(from->si_ptr),
-					    &to->si_ptr);
-				break;
-				 /* This is not generated by the kernel as of now.  */
-			case __SI_RT >> 16:
-			case __SI_MESGQ >> 16:
-				put_user_ex(from->si_uid, &to->si_uid);
-				put_user_ex(from->si_int, &to->si_int);
-				break;
-			}
+			err |= __put_user(from->si_utime,
+				&to->_sifields._sigchld_x32._utime);
+			err |= __put_user(from->si_stime,
+				&to->_sifields._sigchld_x32._stime);
 		}
-	} put_user_catch(err);
-
+		break;
+	case __SI_RT: /* This is not generated by the kernel as of now. */
+	case __SI_MESGQ: /* But this is */
+		err |= __put_user(from->si_pid, &to->si_pid);
+		err |= __put_user(from->si_uid, &to->si_uid);
+		/*
+		 * Get the sigval from si_int, which matches the convention
+		 * used in get_compat_sigevent.
+		 */
+		err |= __put_user(from->si_int, &to->si_int);
+		break;
+#ifdef __ARCH_SIGSYS
+	case __SI_SYS:
+		err |= __put_user(ptr_to_compat(from->si_call_addr),
+			&to->si_call_addr);
+		err |= __put_user(from->si_syscall, &to->si_syscall);
+		err |= __put_user(from->si_arch, &to->si_arch);
+		break;
+#endif
+	default: /* this is just in case for now ... */
+		err |= __put_user(from->si_pid, &to->si_pid);
+		err |= __put_user(from->si_uid, &to->si_uid);
+		break;
+	}
 	return err;
 }
 
 int copy_siginfo_from_user32(siginfo_t *to, compat_siginfo_t __user *from)
 {
-	int err = 0;
-	u32 ptr32;
+	int err;
+	compat_uptr_t ptr32;
+	bool ia32 = test_thread_flag(TIF_IA32);
 
 	if (!access_ok(VERIFY_READ, from, sizeof(compat_siginfo_t)))
 		return -EFAULT;
 
-	get_user_try {
-		get_user_ex(to->si_signo, &from->si_signo);
-		get_user_ex(to->si_errno, &from->si_errno);
-		get_user_ex(to->si_code, &from->si_code);
-
-		get_user_ex(to->si_pid, &from->si_pid);
-		get_user_ex(to->si_uid, &from->si_uid);
-		get_user_ex(ptr32, &from->si_ptr);
-		to->si_ptr = compat_ptr(ptr32);
-	} get_user_catch(err);
-
+	/*
+	 * If you change siginfo_t structure, please be sure that
+	 * all these functions are fixed accordingly:
+	 * copy_siginfo_to_user
+	 * copy_siginfo_to_user32
+	 * copy_siginfo_from_user32
+	 * signalfd_copyinfo
+	 * They should never copy any pad contained in the structure
+	 * to avoid security leaks, but must copy the generic
+	 * 3 ints plus the relevant union member.
+	 */
+	err = __get_user(to->si_signo, &from->si_signo);
+	err |= __get_user(to->si_errno, &from->si_errno);
+	err |= __get_user(to->si_code, &from->si_code);
+	if (to->si_code < 0) {
+		/*
+		 * Note that the compat union may be larger than the normal one
+		 * due to alignment. We work around this by copying any data
+		 * that doesn't fit in the normal union into the padding before
+		 * the union.
+		 */
+		BUILD_BUG_ON(sizeof(to->_sifields._pad) + sizeof(to->_pad2) !=
+			sizeof(from->_sifields._pad));
+		err |= __copy_from_user(to->_sifields._pad,
+			from->_sifields._pad,
+			sizeof(to->_sifields._pad)) ? -EFAULT : 0;
+		err |= __copy_from_user(to->_pad2,
+			from->_sifields._pad + SI_PAD_SIZE, sizeof(to->_pad2))
+			? -EFAULT : 0;
+		return err;
+	}
+	switch (to->si_code & __SI_MASK) {
+	case __SI_KILL:
+		err |= __get_user(to->si_pid, &from->si_pid);
+		err |= __get_user(to->si_uid, &from->si_uid);
+		break;
+	case __SI_TIMER:
+		err |= __get_user(to->si_tid, &from->si_tid);
+		err |= __get_user(to->si_overrun, &from->si_overrun);
+		/*
+		 * Put the sigval in si_int, which matches the convention
+		 * used in get_compat_sigevent.
+		 */
+		to->si_ptr = NULL; /* Avoid uninitialized bits in the union */
+		err |= __get_user(to->si_int, &from->si_int);
+		break;
+	case __SI_POLL:
+		err |= __get_user(to->si_band, &from->si_band);
+		err |= __get_user(to->si_fd, &from->si_fd);
+		break;
+	case __SI_FAULT:
+		err |= __get_user(ptr32, &from->si_addr);
+		to->si_addr = compat_ptr(ptr32);
+#ifdef __ARCH_SI_TRAPNO
+		err |= __get_user(to->si_trapno, &from->si_trapno);
+#endif
+		err |= __get_user(to->si_addr_lsb, &from->si_addr_lsb);
+		err |= __get_user(ptr32, &from->si_lower);
+		to->si_lower = compat_ptr(ptr32);
+		err |= __get_user(ptr32, &from->si_upper);
+		to->si_upper = compat_ptr(ptr32);
+		break;
+	case __SI_CHLD:
+		err |= __get_user(to->si_pid, &from->si_pid);
+		err |= __get_user(to->si_uid, &from->si_uid);
+		err |= __get_user(to->si_status, &from->si_status);
+		if (ia32) {
+			err |= __get_user(to->si_utime, &from->si_utime);
+			err |= __get_user(to->si_stime, &from->si_stime);
+		} else {
+			err |= __get_user(to->si_utime,
+				&from->_sifields._sigchld_x32._utime);
+			err |= __get_user(to->si_stime,
+				&from->_sifields._sigchld_x32._stime);
+		}
+		break;
+	case __SI_RT: /* This is not generated by the kernel as of now. */
+	case __SI_MESGQ: /* But this is */
+		err |= __get_user(to->si_pid, &from->si_pid);
+		err |= __get_user(to->si_uid, &from->si_uid);
+		/*
+		 * Put the sigval in si_int, which matches the convention
+		 * used in get_compat_sigevent.
+		 */
+		to->si_ptr = NULL; /* Avoid uninitialized bits in the union */
+		err |= __get_user(to->si_int, &from->si_int);
+		break;
+#ifdef __ARCH_SIGSYS
+	case __SI_SYS:
+		err |= __get_user(ptr32, &from->si_call_addr);
+		to->si_call_addr = compat_ptr(ptr32);
+		err |= __get_user(to->si_syscall, &from->si_syscall);
+		err |= __get_user(to->si_arch, &from->si_arch);
+		break;
+#endif
+	default: /* this is just in case for now ... */
+		err |= __get_user(to->si_pid, &from->si_pid);
+		err |= __get_user(to->si_uid, &from->si_uid);
+		break;
+	}
 	return err;
 }
-- 
2.6.2


  parent reply	other threads:[~2015-11-05  0:51 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-05  0:50 [PATCH v2 00/20] Fix handling of compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 01/20] compat: Add generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 02/20] compat: Add generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 03/20] x86: Update compat_siginfo_t to be closer to the generic version Amanieu d'Antras
2015-11-05  0:50 ` Amanieu d'Antras [this message]
2015-11-05  2:29   ` [PATCH v2 04/20] x86: Rewrite copy_siginfo_{to,from}_user32 H. Peter Anvin
2015-11-05  0:50 ` [PATCH v2 05/20] mips: Clean up compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 06/20] mips: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 07/20] arm64: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 08/20] arm64: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 09/20] parisc: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 10/20] parsic: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 11/20] s390: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 12/20] s390: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 13/20] powerpc: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 14/20] powerpc: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 15/20] tile: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 16/20] tile: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 17/20] sparc: Use generic compat_siginfo_t Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 18/20] sparc: Use generic copy_siginfo_{to,from}_user32 Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 19/20] signalfd: Fix some issues in signalfd_copyinfo Amanieu d'Antras
2015-11-05  0:50 ` [PATCH v2 20/20] signal: Remove unnecessary zero-initialization of siginfo_t Amanieu d'Antras
2015-11-08  5:09 ` [PATCH v2 00/20] Fix handling of compat_siginfo_t Andy Lutomirski
2015-11-09 15:12   ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1446684640-4112-5-git-send-email-amanieu@gmail.com \
    --to=amanieu@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).