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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no 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 483CDC2D0A8 for ; Sat, 26 Sep 2020 21:10:27 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 981B321D7F for ; Sat, 26 Sep 2020 21:10:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 981B321D7F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arndb.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id BA6546B005C; Sat, 26 Sep 2020 17:10:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B30E06B005D; Sat, 26 Sep 2020 17:10:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A1E7A6B0068; Sat, 26 Sep 2020 17:10:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0178.hostedemail.com [216.40.44.178]) by kanga.kvack.org (Postfix) with ESMTP id 8752B6B005C for ; Sat, 26 Sep 2020 17:10:25 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 32501180AD801 for ; Sat, 26 Sep 2020 21:10:25 +0000 (UTC) X-FDA: 77306456010.26.time47_2a0635227173 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin26.hostedemail.com (Postfix) with ESMTP id 0DE971804A301 for ; Sat, 26 Sep 2020 21:10:25 +0000 (UTC) X-HE-Tag: time47_2a0635227173 X-Filterd-Recvd-Size: 5167 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.24]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Sat, 26 Sep 2020 21:10:24 +0000 (UTC) Received: from mail-qt1-f180.google.com ([209.85.160.180]) by mrelayeu.kundenserver.de (mreue106 [212.227.15.145]) with ESMTPSA (Nemesis) id 1MfZ9C-1kxY9835lw-00g3XZ for ; Sat, 26 Sep 2020 23:10:22 +0200 Received: by mail-qt1-f180.google.com with SMTP id n10so5321659qtv.3 for ; Sat, 26 Sep 2020 14:10:22 -0700 (PDT) X-Gm-Message-State: AOAM530mrC5RiZ1Z0k0JuoCLwrX59/GuptvNSxxMTlxfblCerAc62Wur 3bhY41e55cbnr+2yIhZrSRHFTEqVf/bVBbC7t8o= X-Google-Smtp-Source: ABdhPJwDvlcajU+PF+fBlH/TL4sfEgn4CddfjWtp5EuFd2UBtqo7vjS2hC6ft8mwmLl/Kat4R/mEqJDY8LjXNk3pBI0= X-Received: by 2002:aed:2ce5:: with SMTP id g92mr5933813qtd.204.1601154621203; Sat, 26 Sep 2020 14:10:21 -0700 (PDT) MIME-Version: 1.0 References: <20200918132439.1475479-1-arnd@arndb.de> <20200918132439.1475479-3-arnd@arndb.de> <20200919053716.GJ30063@infradead.org> In-Reply-To: <20200919053716.GJ30063@infradead.org> From: Arnd Bergmann Date: Sat, 26 Sep 2020 23:10:05 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/4] kexec: remove compat_sys_kexec_load syscall To: Christoph Hellwig Cc: Alexander Viro , Eric Biederman , Andrew Morton , "linux-kernel@vger.kernel.org" , Linux ARM , linux-arch , Linux-MM , kexec@lists.infradead.org Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:pCONgDEK8YVgtksicW20aEjgERurjumzfuTvHd4i5Tm6qR099LJ 25Xxh+xd60nqW3AV5+gjF7jMTYH96HNgWfTUxmS7t5BzLzh9cqqNJqkJFgFpKKNJuXnK6x1 GDUK5++6zkor9tAkL1nAz5KsMoFIpJKbmpG7Fa7YCMHgE02lcNlF1Q/te6w2m1b6UggsJ8V JJYB8HZPiGcYRRyecj3aw== X-UI-Out-Filterresults: notjunk:1;V03:K0:LbJZX6CAM9Y=:kwOUNM+zZE2fQC537+AaAo g/TaVU2AjCGe45i053UKclwxLm1xprAw65kiVHyaPe8O30tDn6plAzcZpstOkd9SNXVwWxtD0 oOPcVMRjNDJsBiXgyVf8lG2C7/l8172sLVNZ1jTyNmGhyDnNEWSPQ/x89x5+8jjEDjQY2WtcL 2oiVoBULddLWtpcvktHkNbOBPMs6FFJZtX1kH/hlU1MTM3AeB2BUFcV+HauaYzr8ppZfdjvoe 2K6TdfU1jEvO5WX+Hi//vYpFWL42ZOGkAH2Gp3K6DecDMjvhBV5ysuw4pthNOtaE6ZX3GFyQy LdXFbtGKbdu7VD1g33JyozBJi2R3+uMN4p5CpcTxlz+kcSFKE1zTSR8YLAAlo947hoXf0PBqx 6E+/whxBJGN74p4hefMV/7yvj84b0onQ30rahvJ/w/QEL1F3nIj342bah5vBfD2FzAhlj+dJL LoqRRvGTxwNKz0w8x6UxRd0HFdCpM2JNAl8bH/jo6wJRQhKD2qGifHmfV8gw2bJ+VYH5a41gS z6xjRzVoyQSWMiRnMamYU0b+rKUcxdgxfzWhwTeEsUkuVA6SPN/5L5hv2GzSMJGlsiH7LgKi0 TL1L+29BNlpIVojBgDTJLTgdtdSuFA+tZ4M9f9+ka9FuL/4398yyblYN7kyMDTFlj31l3vspW Zq7rbTkzWWr5xCPYeNRyAZgGMEViS+Z9f3Sd1oP/DLGJ7MPRCJFN5J6qe8PMD+pP8/dUCh8nq mp4yL2VpheKykXFA58xix8URyZ2SoqcRzu146t6DZQtRdx8L5j5o95fJ1eEuxWyNj7AyXv9tA VUOJuyUSqvfg9FyteRyHCJ09guvQOem/1GTTFxbyGZolajlVH2MB7CwXWbF2Z7oKf/4NnZ6vu h4anG63E0T+m5dR4lhdVUH+Yd0n2adhzwBnz1+/zn5FPFWEXhXjCz94vReCPtvAXnX00ARUST D0OAJDjax41OQzbnrAH0c52TVa6BEPq0i3FeRzfL1NsW9JGcDahfJ 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: On Sat, Sep 19, 2020 at 7:37 AM Christoph Hellwig wrote: > > + struct compat_kexec_segment __user *cs = (void __user *)segments; > > + struct compat_kexec_segment segment; > > + int i; > > + for (i=0; i< nr_segments; i++) { > > Missing empty line after the variable declarations and really strange > indentation. > > > + copy_from_user(&segment, &cs[i], sizeof(segment)); > > Missing return value check. > > > + if (ret) > > + break; > > + > > + image->segment[i] = (struct kexec_segment) { > > + .buf = compat_ptr(segment.buf), > > + .bufsz = segment.bufsz, > > + .mem = segment.mem, > > + .memsz = segment.memsz, > > + }; > > + } > > I'd split the whole compat handling into a helper, and I'd probably > use the unsafe_get/put user to optimize it a little more. > > > + } else { > > + ret = copy_from_user(image->segment, segments, segment_bytes); > > + } > > if (ret) > > ret = -EFAULT; > > Why not just > > if (copy_from_user(image->segment, segments, segment_bytes)) > ret = -EFAULT; > > ? Addressed all of these now, thanks for the suggestions! I had already fixed the missing error handling after the kbuild bot pointed that out. The separate function does improve the error handling. I ended up not using unsafe_get/put since I find the copy_from_user based loop more readable and it should lead to smaller object code in most cases as well. kexec is not performance critical, so readability seems more important here. Arnd