Le 17/04/2020 à 23:09, Eric W. Biederman a écrit :

To remove the use of set_fs in the coredump code there needs to be a
way to convert a kernel siginfo to a userspace compat siginfo.

Call that function copy_siginfo_to_compat and factor it out of
copy_siginfo_to_user32.

I find it a pitty to do that.

The existing function could have been easily converted to using user_access_begin() + user_access_end() and use unsafe_put_user() to copy to userspace to avoid copying through a temporary structure on the stack.

With your change, it becomes impossible to do that.

Is that really an issue to use that set_fs() in the coredump code ?

Christophe


The existence of x32 complicates this code.  On x32 SIGCHLD uses 64bit
times for utime and stime.  As only SIGCHLD is affected and SIGCHLD
never causes a coredump I have avoided handling that case.

Signed-off-by: "Eric W. Biederman" <ebied...@xmission.com>
---
  include/linux/compat.h |   1 +
  kernel/signal.c        | 108 +++++++++++++++++++++++------------------
  2 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 0480ba4db592..4962b254e550 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -402,6 +402,7 @@ long compat_get_bitmap(unsigned long *mask, const 
compat_ulong_t __user *umask,
                       unsigned long bitmap_size);
  long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
                       unsigned long bitmap_size);
+void copy_siginfo_to_external32(struct compat_siginfo *to, const struct 
kernel_siginfo *from);
  int copy_siginfo_from_user32(kernel_siginfo_t *to, const struct 
compat_siginfo __user *from);
  int copy_siginfo_to_user32(struct compat_siginfo __user *to, const 
kernel_siginfo_t *from);
  int get_compat_sigevent(struct sigevent *event,
diff --git a/kernel/signal.c b/kernel/signal.c
index e58a6c619824..578f196898cb 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3235,90 +3235,106 @@ int copy_siginfo_from_user(kernel_siginfo_t *to, const 
siginfo_t __user *from)
  }
#ifdef CONFIG_COMPAT
-int copy_siginfo_to_user32(struct compat_siginfo __user *to,
-                          const struct kernel_siginfo *from)
-#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
+void copy_siginfo_to_external32(struct compat_siginfo *to,
+                               const struct kernel_siginfo *from)
  {
-       return __copy_siginfo_to_user32(to, from, in_x32_syscall());
-}
-int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
-                            const struct kernel_siginfo *from, bool x32_ABI)
-#endif
-{
-       struct compat_siginfo new;
-       memset(&new, 0, sizeof(new));
+       /*
+        * This function does not work properly for SIGCHLD on x32,
+        * but it does not need to as SIGCHLD never causes a coredump.
+        */
+       memset(to, 0, sizeof(*to));
- new.si_signo = from->si_signo;
-       new.si_errno = from->si_errno;
-       new.si_code  = from->si_code;
+       to->si_signo = from->si_signo;
+       to->si_errno = from->si_errno;
+       to->si_code  = from->si_code;
        switch(siginfo_layout(from->si_signo, from->si_code)) {
        case SIL_KILL:
-               new.si_pid = from->si_pid;
-               new.si_uid = from->si_uid;
+               to->si_pid = from->si_pid;
+               to->si_uid = from->si_uid;
                break;
        case SIL_TIMER:
-               new.si_tid     = from->si_tid;
-               new.si_overrun = from->si_overrun;
-               new.si_int     = from->si_int;
+               to->si_tid     = from->si_tid;
+               to->si_overrun = from->si_overrun;
+               to->si_int     = from->si_int;
                break;
        case SIL_POLL:
-               new.si_band = from->si_band;
-               new.si_fd   = from->si_fd;
+               to->si_band = from->si_band;
+               to->si_fd   = from->si_fd;
                break;
        case SIL_FAULT:
-               new.si_addr = ptr_to_compat(from->si_addr);
+               to->si_addr = ptr_to_compat(from->si_addr);
  #ifdef __ARCH_SI_TRAPNO
-               new.si_trapno = from->si_trapno;
+               to->si_trapno = from->si_trapno;
  #endif
                break;
        case SIL_FAULT_MCEERR:
-               new.si_addr = ptr_to_compat(from->si_addr);
+               to->si_addr = ptr_to_compat(from->si_addr);
  #ifdef __ARCH_SI_TRAPNO
-               new.si_trapno = from->si_trapno;
+               to->si_trapno = from->si_trapno;
  #endif
-               new.si_addr_lsb = from->si_addr_lsb;
+               to->si_addr_lsb = from->si_addr_lsb;
                break;
        case SIL_FAULT_BNDERR:
-               new.si_addr = ptr_to_compat(from->si_addr);
+               to->si_addr = ptr_to_compat(from->si_addr);
  #ifdef __ARCH_SI_TRAPNO
-               new.si_trapno = from->si_trapno;
+               to->si_trapno = from->si_trapno;
  #endif
-               new.si_lower = ptr_to_compat(from->si_lower);
-               new.si_upper = ptr_to_compat(from->si_upper);
+               to->si_lower = ptr_to_compat(from->si_lower);
+               to->si_upper = ptr_to_compat(from->si_upper);
                break;
        case SIL_FAULT_PKUERR:
-               new.si_addr = ptr_to_compat(from->si_addr);
+               to->si_addr = ptr_to_compat(from->si_addr);
  #ifdef __ARCH_SI_TRAPNO
-               new.si_trapno = from->si_trapno;
+               to->si_trapno = from->si_trapno;
  #endif
-               new.si_pkey = from->si_pkey;
+               to->si_pkey = from->si_pkey;
                break;
        case SIL_CHLD:
-               new.si_pid    = from->si_pid;
-               new.si_uid    = from->si_uid;
-               new.si_status = from->si_status;
+               to->si_pid    = from->si_pid;
+               to->si_uid    = from->si_uid;
+               to->si_status = from->si_status;
+               to->si_utime = from->si_utime;
+               to->si_stime = from->si_stime;
  #ifdef CONFIG_X86_X32_ABI
                if (x32_ABI) {
-                       new._sifields._sigchld_x32._utime = from->si_utime;
-                       new._sifields._sigchld_x32._stime = from->si_stime;
+                       to->_sifields._sigchld_x32._utime = from->si_utime;
+                       to->_sifields._sigchld_x32._stime = from->si_stime;
                } else
  #endif
                {
-                       new.si_utime = from->si_utime;
-                       new.si_stime = from->si_stime;
                }
                break;
        case SIL_RT:
-               new.si_pid = from->si_pid;
-               new.si_uid = from->si_uid;
-               new.si_int = from->si_int;
+               to->si_pid = from->si_pid;
+               to->si_uid = from->si_uid;
+               to->si_int = from->si_int;
                break;
        case SIL_SYS:
-               new.si_call_addr = ptr_to_compat(from->si_call_addr);
-               new.si_syscall   = from->si_syscall;
-               new.si_arch      = from->si_arch;
+               to->si_call_addr = ptr_to_compat(from->si_call_addr);
+               to->si_syscall   = from->si_syscall;
+               to->si_arch      = from->si_arch;
                break;
        }
+}
+
+int copy_siginfo_to_user32(struct compat_siginfo __user *to,
+                          const struct kernel_siginfo *from)
+#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
+{
+       return __copy_siginfo_to_user32(to, from, in_x32_syscall());
+}
+int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
+                            const struct kernel_siginfo *from, bool x32_ABI)
+#endif
+{
+       struct compat_siginfo new;
+       copy_siginfo_to_external32(&new, from);
+#ifdef CONFIG_X86_X32_ABI
+       if (x32_ABI && from->si_signo == SIGCHLD) {
+               new._sifields._sigchld_x32._utime = from->si_utime;
+               new._sifields._sigchld_x32._stime = from->si_stime;
+       }
+#endif
if (copy_to_user(to, &new, sizeof(struct compat_siginfo)))
                return -EFAULT;

Reply via email to