From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BD34C433B4 for ; Tue, 18 May 2021 03:58:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6A20C6124C for ; Tue, 18 May 2021 03:58:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345337AbhERD70 (ORCPT ); Mon, 17 May 2021 23:59:26 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:36684 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242998AbhERD7A (ORCPT ); Mon, 17 May 2021 23:59:00 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrO-001BZ4-7u; Mon, 17 May 2021 21:57:34 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrN-00Dqoh-3t; Mon, 17 May 2021 21:57:33 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Arnd Bergmann Cc: linux-arch@vger.kernel.org, Arnd Bergmann , Christoph Hellwig , Alexander Viro , Andrew Morton , Borislav Petkov , Brian Gerst , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Linux ARM , linux-kernel@vger.kernel.org, Linux-MM , kexec@lists.infradead.org References: <20210517203343.3941777-1-arnd@kernel.org> <20210517203343.3941777-2-arnd@kernel.org> Date: Mon, 17 May 2021 22:57:24 -0500 In-Reply-To: <20210517203343.3941777-2-arnd@kernel.org> (Arnd Bergmann's message of "Mon, 17 May 2021 22:33:40 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1liqrN-00Dqoh-3t;;;mid=;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+4F4J6T7VILKXf+JC2PApdv1NzJS5heMQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Arnd Bergmann writes: > From: Arnd Bergmann > > The compat version of sys_kexec_load() uses compat_alloc_user_space to > convert the user-provided arguments into the native format. > > Move the conversion into the regular implementation with > an in_compat_syscall() check to simplify it and avoid the > compat_alloc_user_space() call. > > compat_sys_kexec_load() now behaves the same as sys_kexec_load(). Is it possible to do this without in_compat_syscall(), and casting pointers to a wrong type? We open ourselves up to bugs whenever we lie to the type system. Skimming through the code it looks like it should be possible to not need the in_compat_syscall and the casts to the wrong type by changing the order of the code a little bit. Eric > Signed-off-by: Arnd Bergmann > --- > include/linux/kexec.h | 2 - > kernel/kexec.c | 95 +++++++++++++++++++------------------------ > 2 files changed, 42 insertions(+), 55 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 0c994ae37729..f61e310d7a85 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -88,14 +88,12 @@ struct kexec_segment { > size_t memsz; > }; > > -#ifdef CONFIG_COMPAT > struct compat_kexec_segment { > compat_uptr_t buf; > compat_size_t bufsz; > compat_ulong_t mem; /* User space sees this as a (void *) ... */ > compat_size_t memsz; > }; > -#endif > > #ifdef CONFIG_KEXEC_FILE > struct purgatory_info { > diff --git a/kernel/kexec.c b/kernel/kexec.c > index c82c6c06f051..6618b1d9f00b 100644 > --- a/kernel/kexec.c > +++ b/kernel/kexec.c > @@ -19,21 +19,46 @@ > > #include "kexec_internal.h" > > +static int copy_user_compat_segment_list(struct kimage *image, > + unsigned long nr_segments, > + void __user *segments) > +{ > + struct compat_kexec_segment __user *cs = segments; > + struct compat_kexec_segment segment; > + int i; > + > + for (i = 0; i < nr_segments; i++) { > + if (copy_from_user(&segment, &cs[i], sizeof(segment))) > + return -EFAULT; > + > + image->segment[i] = (struct kexec_segment) { > + .buf = compat_ptr(segment.buf), > + .bufsz = segment.bufsz, > + .mem = segment.mem, > + .memsz = segment.memsz, > + }; > + } > + > + return 0; > +} > + > + > static int copy_user_segment_list(struct kimage *image, > unsigned long nr_segments, > struct kexec_segment __user *segments) > { > - int ret; > size_t segment_bytes; > > /* Read in the segments */ > image->nr_segments = nr_segments; > segment_bytes = nr_segments * sizeof(*segments); > - ret = copy_from_user(image->segment, segments, segment_bytes); > - if (ret) > - ret = -EFAULT; > + if (in_compat_syscall()) > + return copy_user_compat_segment_list(image, nr_segments, segments); > > - return ret; > + if (copy_from_user(image->segment, segments, segment_bytes)) > + return -EFAULT; > + > + return 0; > } > > static int kimage_alloc_init(struct kimage **rimage, unsigned long entry, > @@ -233,8 +258,9 @@ static inline int kexec_load_check(unsigned long nr_segments, > return 0; > } > > -SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > - struct kexec_segment __user *, segments, unsigned long, flags) > +static int kernel_kexec_load(unsigned long entry, unsigned long nr_segments, > + struct kexec_segment __user * segments, > + unsigned long flags) > { > int result; > > @@ -265,57 +291,20 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > return result; > } > > +SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > + struct kexec_segment __user *, segments, unsigned long, flags) > +{ > + return kernel_kexec_load(entry, nr_segments, segments, flags); > +} > + > #ifdef CONFIG_COMPAT > COMPAT_SYSCALL_DEFINE4(kexec_load, compat_ulong_t, entry, > compat_ulong_t, nr_segments, > struct compat_kexec_segment __user *, segments, > compat_ulong_t, flags) > { > - struct compat_kexec_segment in; > - struct kexec_segment out, __user *ksegments; > - unsigned long i, result; > - > - result = kexec_load_check(nr_segments, flags); > - if (result) > - return result; > - > - /* Don't allow clients that don't understand the native > - * architecture to do anything. > - */ > - if ((flags & KEXEC_ARCH_MASK) == KEXEC_ARCH_DEFAULT) > - return -EINVAL; > - > - ksegments = compat_alloc_user_space(nr_segments * sizeof(out)); > - for (i = 0; i < nr_segments; i++) { > - result = copy_from_user(&in, &segments[i], sizeof(in)); > - if (result) > - return -EFAULT; > - > - out.buf = compat_ptr(in.buf); > - out.bufsz = in.bufsz; > - out.mem = in.mem; > - out.memsz = in.memsz; > - > - result = copy_to_user(&ksegments[i], &out, sizeof(out)); > - if (result) > - return -EFAULT; > - } > - > - /* Because we write directly to the reserved memory > - * region when loading crash kernels we need a mutex here to > - * prevent multiple crash kernels from attempting to load > - * simultaneously, and to prevent a crash kernel from loading > - * over the top of a in use crash kernel. > - * > - * KISS: always take the mutex. > - */ > - if (!mutex_trylock(&kexec_mutex)) > - return -EBUSY; > - > - result = do_kexec_load(entry, nr_segments, ksegments, flags); > - > - mutex_unlock(&kexec_mutex); > - > - return result; > + return kernel_kexec_load(entry, nr_segments, > + (struct kexec_segment __user *)segments, > + flags); > } > #endif From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D41BC433B4 for ; Tue, 18 May 2021 03:57:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 94A2A611AD for ; Tue, 18 May 2021 03:57:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 94A2A611AD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xmission.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D02608E0008; Mon, 17 May 2021 23:57:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CB2C78E0002; Mon, 17 May 2021 23:57:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ADE1F8E0008; Mon, 17 May 2021 23:57:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0079.hostedemail.com [216.40.44.79]) by kanga.kvack.org (Postfix) with ESMTP id 7C1478E0002 for ; Mon, 17 May 2021 23:57:42 -0400 (EDT) Received: from smtpin33.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 0A858AC0F for ; Tue, 18 May 2021 03:57:42 +0000 (UTC) X-FDA: 78152992764.33.2B3EE8E Received: from out03.mta.xmission.com (out03.mta.xmission.com [166.70.13.233]) by imf21.hostedemail.com (Postfix) with ESMTP id 22EC1E0001B2 for ; Tue, 18 May 2021 03:57:40 +0000 (UTC) Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrO-001BZ4-7u; Mon, 17 May 2021 21:57:34 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrN-00Dqoh-3t; Mon, 17 May 2021 21:57:33 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Arnd Bergmann Cc: linux-arch@vger.kernel.org, Arnd Bergmann , Christoph Hellwig , Alexander Viro , Andrew Morton , Borislav Petkov , Brian Gerst , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Linux ARM , linux-kernel@vger.kernel.org, Linux-MM , kexec@lists.infradead.org References: <20210517203343.3941777-1-arnd@kernel.org> <20210517203343.3941777-2-arnd@kernel.org> Date: Mon, 17 May 2021 22:57:24 -0500 In-Reply-To: <20210517203343.3941777-2-arnd@kernel.org> (Arnd Bergmann's message of "Mon, 17 May 2021 22:33:40 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1liqrN-00Dqoh-3t;;;mid=;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+4F4J6T7VILKXf+JC2PApdv1NzJS5heMQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Authentication-Results: imf21.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf21.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.233 as permitted sender) smtp.mailfrom=ebiederm@xmission.com X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 22EC1E0001B2 X-Stat-Signature: ez41izzexn6cnu5ooyj7w3pq3fshxg6q X-HE-Tag: 1621310260-9176 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Arnd Bergmann writes: > From: Arnd Bergmann > > The compat version of sys_kexec_load() uses compat_alloc_user_space to > convert the user-provided arguments into the native format. > > Move the conversion into the regular implementation with > an in_compat_syscall() check to simplify it and avoid the > compat_alloc_user_space() call. > > compat_sys_kexec_load() now behaves the same as sys_kexec_load(). Is it possible to do this without in_compat_syscall(), and casting pointers to a wrong type? We open ourselves up to bugs whenever we lie to the type system. Skimming through the code it looks like it should be possible to not need the in_compat_syscall and the casts to the wrong type by changing the order of the code a little bit. Eric > Signed-off-by: Arnd Bergmann > --- > include/linux/kexec.h | 2 - > kernel/kexec.c | 95 +++++++++++++++++++------------------------ > 2 files changed, 42 insertions(+), 55 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 0c994ae37729..f61e310d7a85 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -88,14 +88,12 @@ struct kexec_segment { > size_t memsz; > }; > > -#ifdef CONFIG_COMPAT > struct compat_kexec_segment { > compat_uptr_t buf; > compat_size_t bufsz; > compat_ulong_t mem; /* User space sees this as a (void *) ... */ > compat_size_t memsz; > }; > -#endif > > #ifdef CONFIG_KEXEC_FILE > struct purgatory_info { > diff --git a/kernel/kexec.c b/kernel/kexec.c > index c82c6c06f051..6618b1d9f00b 100644 > --- a/kernel/kexec.c > +++ b/kernel/kexec.c > @@ -19,21 +19,46 @@ > > #include "kexec_internal.h" > > +static int copy_user_compat_segment_list(struct kimage *image, > + unsigned long nr_segments, > + void __user *segments) > +{ > + struct compat_kexec_segment __user *cs = segments; > + struct compat_kexec_segment segment; > + int i; > + > + for (i = 0; i < nr_segments; i++) { > + if (copy_from_user(&segment, &cs[i], sizeof(segment))) > + return -EFAULT; > + > + image->segment[i] = (struct kexec_segment) { > + .buf = compat_ptr(segment.buf), > + .bufsz = segment.bufsz, > + .mem = segment.mem, > + .memsz = segment.memsz, > + }; > + } > + > + return 0; > +} > + > + > static int copy_user_segment_list(struct kimage *image, > unsigned long nr_segments, > struct kexec_segment __user *segments) > { > - int ret; > size_t segment_bytes; > > /* Read in the segments */ > image->nr_segments = nr_segments; > segment_bytes = nr_segments * sizeof(*segments); > - ret = copy_from_user(image->segment, segments, segment_bytes); > - if (ret) > - ret = -EFAULT; > + if (in_compat_syscall()) > + return copy_user_compat_segment_list(image, nr_segments, segments); > > - return ret; > + if (copy_from_user(image->segment, segments, segment_bytes)) > + return -EFAULT; > + > + return 0; > } > > static int kimage_alloc_init(struct kimage **rimage, unsigned long entry, > @@ -233,8 +258,9 @@ static inline int kexec_load_check(unsigned long nr_segments, > return 0; > } > > -SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > - struct kexec_segment __user *, segments, unsigned long, flags) > +static int kernel_kexec_load(unsigned long entry, unsigned long nr_segments, > + struct kexec_segment __user * segments, > + unsigned long flags) > { > int result; > > @@ -265,57 +291,20 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > return result; > } > > +SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > + struct kexec_segment __user *, segments, unsigned long, flags) > +{ > + return kernel_kexec_load(entry, nr_segments, segments, flags); > +} > + > #ifdef CONFIG_COMPAT > COMPAT_SYSCALL_DEFINE4(kexec_load, compat_ulong_t, entry, > compat_ulong_t, nr_segments, > struct compat_kexec_segment __user *, segments, > compat_ulong_t, flags) > { > - struct compat_kexec_segment in; > - struct kexec_segment out, __user *ksegments; > - unsigned long i, result; > - > - result = kexec_load_check(nr_segments, flags); > - if (result) > - return result; > - > - /* Don't allow clients that don't understand the native > - * architecture to do anything. > - */ > - if ((flags & KEXEC_ARCH_MASK) == KEXEC_ARCH_DEFAULT) > - return -EINVAL; > - > - ksegments = compat_alloc_user_space(nr_segments * sizeof(out)); > - for (i = 0; i < nr_segments; i++) { > - result = copy_from_user(&in, &segments[i], sizeof(in)); > - if (result) > - return -EFAULT; > - > - out.buf = compat_ptr(in.buf); > - out.bufsz = in.bufsz; > - out.mem = in.mem; > - out.memsz = in.memsz; > - > - result = copy_to_user(&ksegments[i], &out, sizeof(out)); > - if (result) > - return -EFAULT; > - } > - > - /* Because we write directly to the reserved memory > - * region when loading crash kernels we need a mutex here to > - * prevent multiple crash kernels from attempting to load > - * simultaneously, and to prevent a crash kernel from loading > - * over the top of a in use crash kernel. > - * > - * KISS: always take the mutex. > - */ > - if (!mutex_trylock(&kexec_mutex)) > - return -EBUSY; > - > - result = do_kexec_load(entry, nr_segments, ksegments, flags); > - > - mutex_unlock(&kexec_mutex); > - > - return result; > + return kernel_kexec_load(entry, nr_segments, > + (struct kexec_segment __user *)segments, > + flags); > } > #endif From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A42EC433B4 for ; Tue, 18 May 2021 04:00:19 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B1638611AD for ; Tue, 18 May 2021 04:00:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B1638611AD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xmission.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:MIME-Version:Message-ID:In-Reply-To:Date: References:Cc:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0V7las8PKwq5QhPjbb2yCZ35IIJL1dTl8llO3SnPnek=; b=CDwn2YRW6qnd2WpWeiO35h0jQ udkpx7LkO+h8vslYM6mi+Jcb7wBd42P5Xy69crpqkHcG816OdiuknK08K05M5yAWYmpEX00upyE38 /P+GqUaFeCbOIHY3epNjBJPlVKSiUsMcvvHCnJTWC+j+oD0O4vLrjvYdNEe0ZQ608A80svbE9nc0J LbN+cXfKWD54PEgv9uiOVjvXxVn8E40m2mzgVMBz9bf2tGc+NA4ngT3qcUmG4/5TnOPpPKCIU8y+B /kBjdOX9raVjzFXM2mGFdUAuTZxIqRQCKfIPbp5lHKfV7AHK1l1kHJLYNFMiH7hL7m360DyPziQrB nvu7rpdQA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1liqrd-00GtUv-Cg; Tue, 18 May 2021 03:57:49 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1liqrY-00GtUN-2Q; Tue, 18 May 2021 03:57:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Subject:Content-Type:MIME-Version: Message-ID:In-Reply-To:Date:References:Cc:To:From:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=MywS8hBNEZcywctNBI+XfUmBkE0MJYsFlJlPKJdxTmg=; b=OL0oVvZ2p4oFbGs6SP1xpOoh5P tZvzqp4OohK+c5vQZI8RBRYK1eUgZK0wEsJUSJkuY4BKcmBcD7AjLMLngyI582Q2D5hHcEFsTakRC 6N/I6vGSJKLA4fX49vf2ncS6fkMTvrZed/+l+pFyWF4cHxwE7NvtVsBwnOdMwDbXtXdVn9gy0RTMM qdavuyN0zTWjRzRxcjx8k7hf95vhxTGulsqjaRXB6D1m4oh7+xfUmMOnynyH38qKn5mukQCEiqP5g xlheyFDurmiAB3ztFeExIxLKLlGAzH8JG7NAvZLOsi4hNsBbn4D8L1kB0Z4x6BqPwwVrVcu+vLOjB MfU1CwSA==; Received: from out03.mta.xmission.com ([166.70.13.233]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1liqrU-00EI3G-SF; Tue, 18 May 2021 03:57:42 +0000 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrO-001BZ4-7u; Mon, 17 May 2021 21:57:34 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1liqrN-00Dqoh-3t; Mon, 17 May 2021 21:57:33 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Arnd Bergmann Cc: linux-arch@vger.kernel.org, Arnd Bergmann , Christoph Hellwig , Alexander Viro , Andrew Morton , Borislav Petkov , Brian Gerst , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Linux ARM , linux-kernel@vger.kernel.org, Linux-MM , kexec@lists.infradead.org References: <20210517203343.3941777-1-arnd@kernel.org> <20210517203343.3941777-2-arnd@kernel.org> Date: Mon, 17 May 2021 22:57:24 -0500 In-Reply-To: <20210517203343.3941777-2-arnd@kernel.org> (Arnd Bergmann's message of "Mon, 17 May 2021 22:33:40 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1liqrN-00Dqoh-3t; ; ; mid=; ; ; hst=in02.mta.xmission.com; ; ; ip=68.227.160.95; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX1+4F4J6T7VILKXf+JC2PApdv1NzJS5heMQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210517_205740_958418_1F3F026D X-CRM114-Status: GOOD ( 32.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Arnd Bergmann writes: > From: Arnd Bergmann > > The compat version of sys_kexec_load() uses compat_alloc_user_space to > convert the user-provided arguments into the native format. > > Move the conversion into the regular implementation with > an in_compat_syscall() check to simplify it and avoid the > compat_alloc_user_space() call. > > compat_sys_kexec_load() now behaves the same as sys_kexec_load(). Is it possible to do this without in_compat_syscall(), and casting pointers to a wrong type? We open ourselves up to bugs whenever we lie to the type system. Skimming through the code it looks like it should be possible to not need the in_compat_syscall and the casts to the wrong type by changing the order of the code a little bit. Eric > Signed-off-by: Arnd Bergmann > --- > include/linux/kexec.h | 2 - > kernel/kexec.c | 95 +++++++++++++++++++------------------------ > 2 files changed, 42 insertions(+), 55 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 0c994ae37729..f61e310d7a85 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -88,14 +88,12 @@ struct kexec_segment { > size_t memsz; > }; > > -#ifdef CONFIG_COMPAT > struct compat_kexec_segment { > compat_uptr_t buf; > compat_size_t bufsz; > compat_ulong_t mem; /* User space sees this as a (void *) ... */ > compat_size_t memsz; > }; > -#endif > > #ifdef CONFIG_KEXEC_FILE > struct purgatory_info { > diff --git a/kernel/kexec.c b/kernel/kexec.c > index c82c6c06f051..6618b1d9f00b 100644 > --- a/kernel/kexec.c > +++ b/kernel/kexec.c > @@ -19,21 +19,46 @@ > > #include "kexec_internal.h" > > +static int copy_user_compat_segment_list(struct kimage *image, > + unsigned long nr_segments, > + void __user *segments) > +{ > + struct compat_kexec_segment __user *cs = segments; > + struct compat_kexec_segment segment; > + int i; > + > + for (i = 0; i < nr_segments; i++) { > + if (copy_from_user(&segment, &cs[i], sizeof(segment))) > + return -EFAULT; > + > + image->segment[i] = (struct kexec_segment) { > + .buf = compat_ptr(segment.buf), > + .bufsz = segment.bufsz, > + .mem = segment.mem, > + .memsz = segment.memsz, > + }; > + } > + > + return 0; > +} > + > + > static int copy_user_segment_list(struct kimage *image, > unsigned long nr_segments, > struct kexec_segment __user *segments) > { > - int ret; > size_t segment_bytes; > > /* Read in the segments */ > image->nr_segments = nr_segments; > segment_bytes = nr_segments * sizeof(*segments); > - ret = copy_from_user(image->segment, segments, segment_bytes); > - if (ret) > - ret = -EFAULT; > + if (in_compat_syscall()) > + return copy_user_compat_segment_list(image, nr_segments, segments); > > - return ret; > + if (copy_from_user(image->segment, segments, segment_bytes)) > + return -EFAULT; > + > + return 0; > } > > static int kimage_alloc_init(struct kimage **rimage, unsigned long entry, > @@ -233,8 +258,9 @@ static inline int kexec_load_check(unsigned long nr_segments, > return 0; > } > > -SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > - struct kexec_segment __user *, segments, unsigned long, flags) > +static int kernel_kexec_load(unsigned long entry, unsigned long nr_segments, > + struct kexec_segment __user * segments, > + unsigned long flags) > { > int result; > > @@ -265,57 +291,20 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > return result; > } > > +SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > + struct kexec_segment __user *, segments, unsigned long, flags) > +{ > + return kernel_kexec_load(entry, nr_segments, segments, flags); > +} > + > #ifdef CONFIG_COMPAT > COMPAT_SYSCALL_DEFINE4(kexec_load, compat_ulong_t, entry, > compat_ulong_t, nr_segments, > struct compat_kexec_segment __user *, segments, > compat_ulong_t, flags) > { > - struct compat_kexec_segment in; > - struct kexec_segment out, __user *ksegments; > - unsigned long i, result; > - > - result = kexec_load_check(nr_segments, flags); > - if (result) > - return result; > - > - /* Don't allow clients that don't understand the native > - * architecture to do anything. > - */ > - if ((flags & KEXEC_ARCH_MASK) == KEXEC_ARCH_DEFAULT) > - return -EINVAL; > - > - ksegments = compat_alloc_user_space(nr_segments * sizeof(out)); > - for (i = 0; i < nr_segments; i++) { > - result = copy_from_user(&in, &segments[i], sizeof(in)); > - if (result) > - return -EFAULT; > - > - out.buf = compat_ptr(in.buf); > - out.bufsz = in.bufsz; > - out.mem = in.mem; > - out.memsz = in.memsz; > - > - result = copy_to_user(&ksegments[i], &out, sizeof(out)); > - if (result) > - return -EFAULT; > - } > - > - /* Because we write directly to the reserved memory > - * region when loading crash kernels we need a mutex here to > - * prevent multiple crash kernels from attempting to load > - * simultaneously, and to prevent a crash kernel from loading > - * over the top of a in use crash kernel. > - * > - * KISS: always take the mutex. > - */ > - if (!mutex_trylock(&kexec_mutex)) > - return -EBUSY; > - > - result = do_kexec_load(entry, nr_segments, ksegments, flags); > - > - mutex_unlock(&kexec_mutex); > - > - return result; > + return kernel_kexec_load(entry, nr_segments, > + (struct kexec_segment __user *)segments, > + flags); > } > #endif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: From: ebiederm@xmission.com (Eric W. Biederman) References: <20210517203343.3941777-1-arnd@kernel.org> <20210517203343.3941777-2-arnd@kernel.org> Date: Mon, 17 May 2021 22:57:24 -0500 In-Reply-To: <20210517203343.3941777-2-arnd@kernel.org> (Arnd Bergmann's message of "Mon, 17 May 2021 22:33:40 +0200") Message-ID: MIME-Version: 1.0 Subject: Re: [PATCH v3 1/4] kexec: simplify compat_sys_kexec_load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Arnd Bergmann Cc: linux-arch@vger.kernel.org, Arnd Bergmann , Christoph Hellwig , Alexander Viro , Andrew Morton , Borislav Petkov , Brian Gerst , Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Linux ARM , linux-kernel@vger.kernel.org, Linux-MM , kexec@lists.infradead.org Arnd Bergmann writes: > From: Arnd Bergmann > > The compat version of sys_kexec_load() uses compat_alloc_user_space to > convert the user-provided arguments into the native format. > > Move the conversion into the regular implementation with > an in_compat_syscall() check to simplify it and avoid the > compat_alloc_user_space() call. > > compat_sys_kexec_load() now behaves the same as sys_kexec_load(). Is it possible to do this without in_compat_syscall(), and casting pointers to a wrong type? We open ourselves up to bugs whenever we lie to the type system. Skimming through the code it looks like it should be possible to not need the in_compat_syscall and the casts to the wrong type by changing the order of the code a little bit. Eric > Signed-off-by: Arnd Bergmann > --- > include/linux/kexec.h | 2 - > kernel/kexec.c | 95 +++++++++++++++++++------------------------ > 2 files changed, 42 insertions(+), 55 deletions(-) > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h > index 0c994ae37729..f61e310d7a85 100644 > --- a/include/linux/kexec.h > +++ b/include/linux/kexec.h > @@ -88,14 +88,12 @@ struct kexec_segment { > size_t memsz; > }; > > -#ifdef CONFIG_COMPAT > struct compat_kexec_segment { > compat_uptr_t buf; > compat_size_t bufsz; > compat_ulong_t mem; /* User space sees this as a (void *) ... */ > compat_size_t memsz; > }; > -#endif > > #ifdef CONFIG_KEXEC_FILE > struct purgatory_info { > diff --git a/kernel/kexec.c b/kernel/kexec.c > index c82c6c06f051..6618b1d9f00b 100644 > --- a/kernel/kexec.c > +++ b/kernel/kexec.c > @@ -19,21 +19,46 @@ > > #include "kexec_internal.h" > > +static int copy_user_compat_segment_list(struct kimage *image, > + unsigned long nr_segments, > + void __user *segments) > +{ > + struct compat_kexec_segment __user *cs = segments; > + struct compat_kexec_segment segment; > + int i; > + > + for (i = 0; i < nr_segments; i++) { > + if (copy_from_user(&segment, &cs[i], sizeof(segment))) > + return -EFAULT; > + > + image->segment[i] = (struct kexec_segment) { > + .buf = compat_ptr(segment.buf), > + .bufsz = segment.bufsz, > + .mem = segment.mem, > + .memsz = segment.memsz, > + }; > + } > + > + return 0; > +} > + > + > static int copy_user_segment_list(struct kimage *image, > unsigned long nr_segments, > struct kexec_segment __user *segments) > { > - int ret; > size_t segment_bytes; > > /* Read in the segments */ > image->nr_segments = nr_segments; > segment_bytes = nr_segments * sizeof(*segments); > - ret = copy_from_user(image->segment, segments, segment_bytes); > - if (ret) > - ret = -EFAULT; > + if (in_compat_syscall()) > + return copy_user_compat_segment_list(image, nr_segments, segments); > > - return ret; > + if (copy_from_user(image->segment, segments, segment_bytes)) > + return -EFAULT; > + > + return 0; > } > > static int kimage_alloc_init(struct kimage **rimage, unsigned long entry, > @@ -233,8 +258,9 @@ static inline int kexec_load_check(unsigned long nr_segments, > return 0; > } > > -SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > - struct kexec_segment __user *, segments, unsigned long, flags) > +static int kernel_kexec_load(unsigned long entry, unsigned long nr_segments, > + struct kexec_segment __user * segments, > + unsigned long flags) > { > int result; > > @@ -265,57 +291,20 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > return result; > } > > +SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, > + struct kexec_segment __user *, segments, unsigned long, flags) > +{ > + return kernel_kexec_load(entry, nr_segments, segments, flags); > +} > + > #ifdef CONFIG_COMPAT > COMPAT_SYSCALL_DEFINE4(kexec_load, compat_ulong_t, entry, > compat_ulong_t, nr_segments, > struct compat_kexec_segment __user *, segments, > compat_ulong_t, flags) > { > - struct compat_kexec_segment in; > - struct kexec_segment out, __user *ksegments; > - unsigned long i, result; > - > - result = kexec_load_check(nr_segments, flags); > - if (result) > - return result; > - > - /* Don't allow clients that don't understand the native > - * architecture to do anything. > - */ > - if ((flags & KEXEC_ARCH_MASK) == KEXEC_ARCH_DEFAULT) > - return -EINVAL; > - > - ksegments = compat_alloc_user_space(nr_segments * sizeof(out)); > - for (i = 0; i < nr_segments; i++) { > - result = copy_from_user(&in, &segments[i], sizeof(in)); > - if (result) > - return -EFAULT; > - > - out.buf = compat_ptr(in.buf); > - out.bufsz = in.bufsz; > - out.mem = in.mem; > - out.memsz = in.memsz; > - > - result = copy_to_user(&ksegments[i], &out, sizeof(out)); > - if (result) > - return -EFAULT; > - } > - > - /* Because we write directly to the reserved memory > - * region when loading crash kernels we need a mutex here to > - * prevent multiple crash kernels from attempting to load > - * simultaneously, and to prevent a crash kernel from loading > - * over the top of a in use crash kernel. > - * > - * KISS: always take the mutex. > - */ > - if (!mutex_trylock(&kexec_mutex)) > - return -EBUSY; > - > - result = do_kexec_load(entry, nr_segments, ksegments, flags); > - > - mutex_unlock(&kexec_mutex); > - > - return result; > + return kernel_kexec_load(entry, nr_segments, > + (struct kexec_segment __user *)segments, > + flags); > } > #endif _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec