[PATCH v2 00/20] Fix handling of compat_siginfo_t

[PATCH v2 00/20] Fix handling of compat_siginfo_t

[Amanieu d'Antras]

The current handling of compat_siginfo_t is a mess: each architecture has its
own implementation, all of which are incorrect in different ways. This patch
series replaces all of the arch-specific versions with a single generic one that
is guaranteed to produce the same results as a 32-bit kernel.

Most architectures are able to use the generic compat_siginfo_t, except x86 and
MIPS. MIPS uses a slightly different compat_siginfo_t structure for ABI reasons
but can still use the generic copy_siginfo_{to,from}_user32. x86 can't use the
generic versions because it needs special handling for __SI_CHLD for x32 tasks.

One issue that isn't resolved in this series is sending signals between a 32-bit
process and 64-bit process. Sending a si_int will work correctly, but a si_ptr
value will likely get corrupted due to the different layouts of the 32-bit and
64-bit siginfo_t structures.

signalfd_copyinfo was also modified to properly generate data for compat tasks.
In particular the ssi_ptr and ssi_data members need to be sign-extended to 64
bits rather than zero-extended, since that is the behavior in 32-bit kernels.

This series has been tested on x86_64 and arm64.

Changes since v1:
- Properly copy padding bytes and avoid leaking uninitialized data to userspace
- Fixed compile errors on mips and powerpc
- Fixed some compiler warnings
- Fixed some formatting issues

Amanieu d'Antras (20):
  compat: Add generic compat_siginfo_t
  compat: Add generic copy_siginfo_{to,from}_user32
  x86: Update compat_siginfo_t to be closer to the generic version
  x86: Rewrite copy_siginfo_{to,from}_user32
  mips: Clean up compat_siginfo_t
  mips: Use generic copy_siginfo_{to,from}_user32
  arm64: Use generic compat_siginfo_t
  arm64: Use generic copy_siginfo_{to,from}_user32
  parisc: Use generic compat_siginfo_t
  parsic: Use generic copy_siginfo_{to,from}_user32
  s390: Use generic compat_siginfo_t
  s390: Use generic copy_siginfo_{to,from}_user32
  powerpc: Use generic compat_siginfo_t
  powerpc: Use generic copy_siginfo_{to,from}_user32
  tile: Use generic compat_siginfo_t
  tile: Use generic copy_siginfo_{to,from}_user32
  sparc: Use generic compat_siginfo_t
  sparc: Use generic copy_siginfo_{to,from}_user32
  signalfd: Fix some issues in signalfd_copyinfo
  signal: Remove unnecessary zero-initialization of siginfo_t

 arch/arm64/include/asm/compat.h    |  59 --------
 arch/arm64/kernel/signal32.c       |  85 -----------
 arch/mips/include/asm/compat.h     |  63 ++++----
 arch/mips/kernel/signal32.c        |  62 --------
 arch/parisc/include/asm/compat.h   |  52 -------
 arch/parisc/kernel/signal32.c      | 102 -------------
 arch/powerpc/include/asm/compat.h  |  60 --------
 arch/powerpc/kernel/signal_32.c    |  72 +---------
 arch/s390/include/asm/compat.h     |  51 -------
 arch/s390/kernel/compat_signal.c   | 102 -------------
 arch/sparc/include/asm/compat.h    |  54 -------
 arch/sparc/kernel/signal32.c       |  69 ---------
 arch/tile/include/asm/compat.h     |  57 --------
 arch/tile/kernel/compat_signal.c   |  75 ----------
 arch/x86/include/asm/compat.h      |  39 +++--
 arch/x86/kernel/signal_compat.c    | 285 ++++++++++++++++++++++++++++---------
 fs/signalfd.c                      |  58 +++++---
 include/linux/compat.h             |  66 ++++++++-
 include/uapi/asm-generic/siginfo.h |   1 +
 kernel/compat.c                    | 224 +++++++++++++++++++++++++++++
 kernel/ptrace.c                    |   1 -
 kernel/signal.c                    |  16 ++-
 22 files changed, 615 insertions(+), 1038 deletions(-)

-- 
2.6.2

[PATCH v2 01/20] compat: Add generic compat_siginfo_t

[Amanieu d'Antras]

This matches the normal siginfo_t as closely as possible, unlike
some architecture-specific versions which are missing some fields.

Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
---
 arch/arm64/include/asm/compat.h   |  2 ++
 arch/mips/include/asm/compat.h    |  1 +
 arch/parisc/include/asm/compat.h  |  2 ++
 arch/powerpc/include/asm/compat.h |  1 +
 arch/s390/include/asm/compat.h    |  2 ++
 arch/sparc/include/asm/compat.h   |  1 +
 arch/tile/include/asm/compat.h    |  1 +
 arch/x86/include/asm/compat.h     |  2 ++
 include/linux/compat.h            | 66 ++++++++++++++++++++++++++++++++++++++-
 9 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 7fbed69..ff4e294 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -155,6 +155,8 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
+
 typedef struct compat_siginfo {
 	int si_signo;
 	int si_errno;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index c4bd54a..5f1f816 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -130,6 +130,7 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index 94710cf..e0be05f 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -134,6 +134,8 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
+
 typedef struct compat_siginfo {
 	int si_signo;
 	int si_errno;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 4f2df58..75b25ff 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -124,6 +124,7 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index d350ed9..ac73ac7 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -192,6 +192,8 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
+
 typedef struct compat_siginfo {
 	int	si_signo;
 	int	si_errno;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 830502fe..0c80f59 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -153,6 +153,7 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/tile/include/asm/compat.h b/arch/tile/include/asm/compat.h
index c14e36f..f9bba8d 100644
--- a/arch/tile/include/asm/compat.h
+++ b/arch/tile/include/asm/compat.h
@@ -115,6 +115,7 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
 #define COMPAT_SI_PAD_SIZE	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index acdee09..69176b4 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -130,6 +130,8 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+#define HAVE_ARCH_COMPAT_SIGINFO_T
+
 typedef struct compat_siginfo {
 	int si_signo;
 	int si_errno;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index a76c917..e51574c 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -196,7 +196,71 @@ struct compat_rusage {
 extern int put_compat_rusage(const struct rusage *,
 			     struct compat_rusage __user *);
 
-struct compat_siginfo;
+#ifndef HAVE_ARCH_COMPAT_SIGINFO_T
+typedef struct compat_siginfo {
+	int si_signo;
+	int si_errno;
+	int si_code;
+
+	union {
+		int _pad[128 / sizeof(int) - 3];
+
+		/* kill() */
+		struct {
+			compat_pid_t _pid;	/* sender's pid */
+			compat_uid_t _uid;	/* sender's uid */
+		} _kill;
+
+		/* POSIX.1b timers */
+		struct {
+			compat_timer_t _tid;	/* timer id */
+			int _overrun;		/* overrun count */
+			compat_sigval_t _sigval;	/* same as below */
+		} _timer;
+
+		/* POSIX.1b signals */
+		struct {
+			compat_pid_t _pid;	/* sender's pid */
+			compat_uid_t _uid;	/* sender's uid */
+			compat_sigval_t _sigval;
+		} _rt;
+
+		/* SIGCHLD */
+		struct {
+			compat_pid_t _pid;	/* which child */
+			compat_uid_t _uid;	/* sender's uid */
+			int _status;		/* exit code */
+			compat_clock_t _utime;
+			compat_clock_t _stime;
+		} _sigchld;
+
+		/* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
+		struct {
+			compat_uptr_t _addr;	/* faulting insn/memory ref. */
+#ifdef __ARCH_SI_TRAPNO
+			int _trapno;	/* TRAP # which caused the signal */
+#endif
+			short _addr_lsb; /* LSB of the reported address */
+			struct {
+				compat_uptr_t _lower;
+				compat_uptr_t _upper;
+			} _addr_bnd;
+		} _sigfault;
+
+		/* SIGPOLL */
+		struct {
+			compat_long_t _band; /* POLL_IN, POLL_OUT, POLL_MSG */
+			int _fd;
+		} _sigpoll;
+
+		struct {
+			compat_uptr_t _call_addr; /* calling insn */
+			int _syscall;	/* triggering system call number */
+			compat_uint_t _arch;	/* AUDIT_ARCH_* of syscall */
+		} _sigsys;
+	} _sifields;
+} compat_siginfo_t;
+#endif
 
 extern asmlinkage long compat_sys_waitid(int, compat_pid_t,
 		struct compat_siginfo __user *, int,
-- 
2.6.2

[PATCH v2 02/20] compat: Add generic copy_siginfo_{to,from}_user32

[Amanieu d'Antras]

These routines try to match the behavior of native 32-bit kernels
as closely as possible. They will replace architecture-specific
versions that are missing support for some fields and have various
bugs that cause behavior to diverge from that of a 32-bit kernel.

The only problematic situation is when sending a si_ptr from a
32-bit process to a 64-bit process or vice-versa, but this has
never worked correctly in the past anyways.

One thing to note is that, because the size of the siginfo_t union
differs between 32-bit and 64-bit systems, we need to stash the
last 4 bytes of the union in the 4 bytes of padding between the
64-bit union and the initial 3 siginfo_t members.

Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
---
 arch/arm64/include/asm/compat.h    |   2 +
 arch/mips/include/asm/compat.h     |   2 +
 arch/parisc/include/asm/compat.h   |   2 +
 arch/powerpc/include/asm/compat.h  |   2 +
 arch/s390/include/asm/compat.h     |   2 +
 arch/sparc/include/asm/compat.h    |   2 +
 arch/tile/include/asm/compat.h     |   2 +
 arch/x86/include/asm/compat.h      |   2 +
 include/uapi/asm-generic/siginfo.h |   1 +
 kernel/compat.c                    | 224 +++++++++++++++++++++++++++++++++++++
 kernel/signal.c                    |  12 +-
 11 files changed, 248 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index ff4e294..5eae749 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -156,6 +156,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 
 typedef struct compat_siginfo {
 	int si_signo;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 5f1f816..1e5ba38 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -131,6 +131,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index e0be05f..46a0a8a 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -135,6 +135,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 
 typedef struct compat_siginfo {
 	int si_signo;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 75b25ff..cdc8638 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -125,6 +125,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index ac73ac7..497af62 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -193,6 +193,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 
 typedef struct compat_siginfo {
 	int	si_signo;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 0c80f59..9357014 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -154,6 +154,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 #define SI_PAD_SIZE32	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/tile/include/asm/compat.h b/arch/tile/include/asm/compat.h
index f9bba8d..e0c61da 100644
--- a/arch/tile/include/asm/compat.h
+++ b/arch/tile/include/asm/compat.h
@@ -116,6 +116,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 #define COMPAT_SI_PAD_SIZE	(128/sizeof(int) - 3)
 
 typedef struct compat_siginfo {
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 69176b4..c6b58b1 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -131,6 +131,8 @@ typedef union compat_sigval {
 } compat_sigval_t;
 
 #define HAVE_ARCH_COMPAT_SIGINFO_T
+#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
+#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 
 typedef struct compat_siginfo {
 	int si_signo;
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 1e35520..cc8d95e 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -49,6 +49,7 @@ typedef struct siginfo {
 	int si_signo;
 	int si_errno;
 	int si_code;
+	int _pad2[__ARCH_SI_PREAMBLE_SIZE / sizeof(int) - 3];
 
 	union {
 		int _pad[SI_PAD_SIZE];
diff --git a/kernel/compat.c b/kernel/compat.c
index 333d364..644da25 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -1174,3 +1174,227 @@ void __user *compat_alloc_user_space(unsigned long len)
 	return ptr;
 }
 EXPORT_SYMBOL_GPL(compat_alloc_user_space);
+
+#ifndef HAVE_ARCH_COPY_SIGINFO_TO_USER32
+int copy_siginfo_to_user32(compat_siginfo_t __user *to, const siginfo_t *from)
+{
+	int err, si_code;
+
+	if (!access_ok(VERIFY_WRITE, to, sizeof(compat_siginfo_t)))
+		return -EFAULT;
+
+	/*
+	 * 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 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);
+		err |= __put_user(from->si_utime, &to->si_utime);
+		err |= __put_user(from->si_stime, &to->si_stime);
+		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;
+}
+#endif
+
+#ifndef HAVE_ARCH_COPY_SIGINFO_FROM_USER32
+int copy_siginfo_from_user32(siginfo_t *to, compat_siginfo_t __user *from)
+{
+	int err;
+	compat_uptr_t ptr32;
+
+	if (!access_ok(VERIFY_READ, from, sizeof(compat_siginfo_t)))
+		return -EFAULT;
+
+	/*
+	 * 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);
+		err |= __get_user(to->si_utime, &from->si_utime);
+		err |= __get_user(to->si_stime, &from->si_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;
+}
+#endif
diff --git a/kernel/signal.c b/kernel/signal.c
index 0f6bbbe..873e8e2 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2713,11 +2713,13 @@ int copy_siginfo_to_user(siginfo_t __user *to, const siginfo_t *from)
 		return __copy_to_user(to, from, sizeof(siginfo_t))
 			? -EFAULT : 0;
 	/*
-	 * If you change siginfo_t structure, please be sure
-	 * this code is fixed accordingly.
-	 * Please remember to update the signalfd_copyinfo() function
-	 * inside fs/signalfd.c too, in case siginfo_t changes.
-	 * It should never copy any pad contained in the structure
+	 * 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.
 	 */
-- 
2.6.2

[PATCH v2 07/20] arm64: Use generic compat_siginfo_t

[Amanieu d'Antras]

Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
---
 arch/arm64/include/asm/compat.h | 60 -----------------------------------------
 1 file changed, 60 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 5eae749..dc7cfc1 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -155,69 +155,9 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
-#define HAVE_ARCH_COMPAT_SIGINFO_T
 #define HAVE_ARCH_COPY_SIGINFO_TO_USER32
 #define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
 
-typedef struct compat_siginfo {
-	int si_signo;
-	int si_errno;
-	int si_code;
-
-	union {
-		int _pad[128/sizeof(int) - 3];
-
-		/* kill() */
-		struct {
-			compat_pid_t _pid;	/* sender's pid */
-			__compat_uid32_t _uid;	/* sender's uid */
-		} _kill;
-
-		/* POSIX.1b timers */
-		struct {
-			compat_timer_t _tid;	/* timer id */
-			int _overrun;		/* overrun count */
-			compat_sigval_t _sigval;	/* same as below */
-			int _sys_private;       /* not to be passed to user */
-		} _timer;
-
-		/* POSIX.1b signals */
-		struct {
-			compat_pid_t _pid;	/* sender's pid */
-			__compat_uid32_t _uid;	/* sender's uid */
-			compat_sigval_t _sigval;
-		} _rt;
-
-		/* SIGCHLD */
-		struct {
-			compat_pid_t _pid;	/* which child */
-			__compat_uid32_t _uid;	/* sender's uid */
-			int _status;		/* exit code */
-			compat_clock_t _utime;
-			compat_clock_t _stime;
-		} _sigchld;
-
-		/* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
-		struct {
-			compat_uptr_t _addr; /* faulting insn/memory ref. */
-			short _addr_lsb; /* LSB of the reported address */
-		} _sigfault;
-
-		/* SIGPOLL */
-		struct {
-			compat_long_t _band;	/* POLL_IN, POLL_OUT, POLL_MSG */
-			int _fd;
-		} _sigpoll;
-
-		/* SIGSYS */
-		struct {
-			compat_uptr_t _call_addr; /* calling user insn */
-			int _syscall;	/* triggering system call number */
-			compat_uint_t _arch;	/* AUDIT_ARCH_* of syscall */
-		} _sigsys;
-	} _sifields;
-} compat_siginfo_t;
-
 #define COMPAT_OFF_T_MAX	0x7fffffff
 #define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
-- 
2.6.2

[PATCH v2 08/20] arm64: Use generic copy_siginfo_{to,from}_user32

[Amanieu d'Antras]

Signed-off-by: Amanieu d'Antras <amanieu@gmail.com>
---
 arch/arm64/include/asm/compat.h |  3 --
 arch/arm64/kernel/signal32.c    | 85 -----------------------------------------
 2 files changed, 88 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index dc7cfc1..824be1dd 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -155,9 +155,6 @@ typedef union compat_sigval {
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
-#define HAVE_ARCH_COPY_SIGINFO_TO_USER32
-#define HAVE_ARCH_COPY_SIGINFO_FROM_USER32
-
 #define COMPAT_OFF_T_MAX	0x7fffffff
 #define COMPAT_LOFF_T_MAX	0x7fffffffffffffffL
 
diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c
index 71ef6dc..2a69815 100644
--- a/arch/arm64/kernel/signal32.c
+++ b/arch/arm64/kernel/signal32.c
@@ -125,91 +125,6 @@ static inline int get_sigset_t(sigset_t *set,
 	return 0;
 }
 
-int copy_siginfo_to_user32(compat_siginfo_t __user *to, const siginfo_t *from)
-{
-	int err;
-
-	if (!access_ok(VERIFY_WRITE, to, sizeof(*to)))
-		return -EFAULT;
-
-	/* If you change siginfo_t structure, please be sure
-	 * 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.
-	 * This routine must convert siginfo from 64bit to 32bit as well
-	 * at the same time.
-	 */
-	err = __put_user(from->si_signo, &to->si_signo);
-	err |= __put_user(from->si_errno, &to->si_errno);
-	err |= __put_user((short)from->si_code, &to->si_code);
-	if (from->si_code < 0)
-		err |= __copy_to_user(&to->_sifields._pad, &from->_sifields._pad,
-				      SI_PAD_SIZE);
-	else 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);
-		 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((compat_uptr_t)(unsigned long)from->si_addr,
-				  &to->si_addr);
-#ifdef BUS_MCEERR_AO
-		/*
-		 * Other callers might not initialize the si_lsb field,
-		 * so check explicitely 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
-		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);
-		err |= __put_user(from->si_utime, &to->si_utime);
-		err |= __put_user(from->si_stime, &to->si_stime);
-		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);
-		err |= __put_user(from->si_int, &to->si_int);
-		break;
-	case __SI_SYS:
-		err |= __put_user((compat_uptr_t)(unsigned long)
-				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;
-	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)
-{
-	if (copy_from_user(to, from, __ARCH_SI_PREAMBLE_SIZE) ||
-	    copy_from_user(to->_sifields._pad,
-			   from->_sifields._pad, SI_PAD_SIZE))
-		return -EFAULT;
-
-	return 0;
-}
-
 /*
  * VFP save/restore code.
  *
-- 
2.6.2

[PATCH v2 00/20] Fix handling of compat_siginfo_t

[Andy Lutomirski]

On Wed, Nov 4, 2015 at 4:50 PM, Amanieu d'Antras <amanieu@gmail.com> wrote:
> One issue that isn't resolved in this series is sending signals between a 32-bit
> process and 64-bit process. Sending a si_int will work correctly, but a si_ptr
> value will likely get corrupted due to the different layouts of the 32-bit and
> 64-bit siginfo_t structures.

This is so screwed up it's not even funny.

A 64-bit big-endian compat calls rt_sigqueueinfo.  It passes in (among
other things) a sigval_t.  The kernel can choose to interpret it as a
pointer (call it p) or an integer (call it i).  Then (unsigned long)p
= (i<<32) | [something].  If the number was an integer to begin with
*and* user code zeroed out the mess first, then [something] will be 0.
Regardless, p != i unless they're both zero.

If the result gets delivered to a signalfd, then it's plausible that
everything could work.  If it gets delivered to a 64-bit siginfo, then
all is well because it's in exactly the same screwed up state it was
in when the signal gets sent.

If, however, it's delivered to a compat task, wtf is the kernel
supposed to do?  We're effectively supposed to convert a 64-bit
sigval_t to a 32-bit sigval_t.  On a little-endian architecture, we
can fudge it because it doesn't really matter whether we consider the
pointer or the int to be authoritative.  I think that, on big-endian,
we're screwed.

BTW, x86 has its own set of screwups here.  Somehow cr2 and error_code
ended up as part of ucontext instead of siginfo, which makes
absolutely no sense to me and bloats task_struct.

--Andy

[PATCH v2 00/20] Fix handling of compat_siginfo_t

[Oleg Nesterov]

On 11/07, Andy Lutomirski wrote:
>
> On Wed, Nov 4, 2015 at 4:50 PM, Amanieu d'Antras <amanieu@gmail.com> wrote:
> > One issue that isn't resolved in this series is sending signals between a 32-bit
> > process and 64-bit process. Sending a si_int will work correctly, but a si_ptr
> > value will likely get corrupted due to the different layouts of the 32-bit and
> > 64-bit siginfo_t structures.
>
> This is so screwed up it's not even funny.

Agreed,

> A 64-bit big-endian compat calls rt_sigqueueinfo.  It passes in (among
> other things) a sigval_t.  The kernel can choose to interpret it

I always thought that the kernel should not interpret it at all. And indeed,
copy_siginfo_to_user() does

	if (from->si_code < 0)
		return __copy_to_user(to, from, sizeof(siginfo_t))

probably copy_siginfo_to_user32() should do something similar,@least
it should not truncate ->si_code it it is less than zero.

Not sure what signalfd_copyinfo() should do.

But perhaps I was wrong, I failed to find man sigqueueinfo, and man
sigqueue() documents that it passes sigval_t.


> BTW, x86 has its own set of screwups here.  Somehow cr2 and error_code
> ended up as part of ucontext instead of siginfo, which makes
> absolutely no sense to me and bloats task_struct.

Yes, and probably ->ip should have been the part of siginfo too. Say,
if you get SIGBUS you can't trust sc->ip if another signal was dequeued
before SIGBUS, in this case sc->ip will point to the handler of that
another signal. That is why we have SYNCHRONOUS_MASK and it helps, but
still this doesn't look nice.

Oleg.