| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c51c6192-2ea4-62d8-dd22-305f7a1e0dd3@c-s.fr>
Date: Sat, 18 Apr 2020 10:05:19 +0200
From: Christophe Leroy <christophe.leroy@....fr>
To: "Eric W. Biederman" <ebiederm@...ssion.com>,
Christoph Hellwig <hch@....de>
Cc: Arnd Bergmann <arnd@...db.de>, x86@...nel.org,
linux-kernel@...r.kernel.org,
Alexander Viro <viro@...iv.linux.org.uk>,
linux-fsdevel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
linuxppc-dev@...ts.ozlabs.org, Jeremy Kerr <jk@...abs.org>
Subject: Re: [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from
copy_siginfo_to_user32
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" <ebiederm@...ssion.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;
>
Powered by blists - more mailing lists