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 42A3BC433ED for ; Tue, 18 May 2021 13:43:44 +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 BB4BA610FA for ; Tue, 18 May 2021 13:43:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BB4BA610FA 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=sbentNRO7Cm4lG55HxeHpNZczlgTBsP4W4T88A8+C7E=; b=ejxr5sZA/7IyHKYpe8QYqvM+a fDGrGH8rcrMpEmPxqfD8YJVwK7XMfulA0MHtdd1WcYC7y2ldfbUIIN5T9AZx/KFzfN8XdrUIswIeW lh8awRLrA9Su+PN8BJWUx+A0KBmqHs1xgjxP2PWpIqNUE+elDvhEO5SBhWK+1vq11ZCf8YEXG7J0+ zw52MJYDyHNkmkRUV6KM56c4E09LAqB98VAEkUttmt+fGWSfyP58Dq16UFTSdANrRsP0XMP5hUypH /le3hPaApozYfnI3VGii4E7Q51M0D/mYtfA5ZfxZJZqZbGkcJz5gVF2nIk1pvWW8L2XjEaLvEB/+U usshf6Jmw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lizyX-000tey-88; Tue, 18 May 2021 13:41:33 +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 1lizyS-000teJ-7r; Tue, 18 May 2021 13:41:28 +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=s/k2SSWnBtpJL+Pr7+t1dRiiIno6S5Kyo5aj1mJzPcY=; b=1AKyM62sQCdKUd7h8mJ1lVLwTV ufnSgmurbNMAOi2DcUBGUkn1lllDY1hidlI9njBH/dRi7k6Tp4iUYHFTYxer2RSIMfqMvX+WRxT89 cVkJPJp6mKO1CZ0CvFGYoa9QE1+pQZUCRlC9bEuLCLWQv2VNO0i+57LERrhq3K3MA8sQnHf3O/8S/ WmZsTU7QNGkRO0vyW386MbhhAYrlbXKeFvRZe8NhxXBhNNCR+Vk4iQX6+DxfCKNkfqCgsnZNxrYQR v8yQTS8P0qKCRVO7TdnhTafvKQWdyHU/2Rh2PHMtXGjKMSBlnfUFuBQxUEkKVs7ymUSJdiI9Y7i2Z GNfNAnyw==; Received: from out01.mta.xmission.com ([166.70.13.231]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lizyO-00Eh5i-Oy; Tue, 18 May 2021 13:41:26 +0000 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1lizyG-00Axt3-Vi; Tue, 18 May 2021 07:41:17 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1lizyF-0005tf-St; Tue, 18 May 2021 07:41:16 -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: Tue, 18 May 2021 08:41:07 -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=1lizyF-0005tf-St; ; ; mid=; ; ; hst=in01.mta.xmission.com; ; ; ip=68.227.160.95; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX1+3MixM5wFfwyFMlKmVM10vOLfImi60fNc= 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 Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210518_064124_858442_AA6CC1A3 X-CRM114-Status: GOOD ( 34.76 ) 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(). Nacked-by: "Eric W. Biederman" The patch is wrong. The logic between the compat entry point and the ordinary entry point are by necessity different. This unifies the logic and breaks the compat entry point. The fundamentally necessity is that the code being loaded needs to know which mode the kernel is running in so it can safely transition to the new kernel. Given that the two entry points fundamentally need different logic, and that difference was not preserved and the goal of this patchset was to unify that which fundamentally needs to be different. I don't think this patch series makes any sense for kexec. 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