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=-2.4 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 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 1ABA9C38A30 for ; Sun, 19 Apr 2020 09:54:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E2B8E21473 for ; Sun, 19 Apr 2020 09:54:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=c-s.fr header.i=@c-s.fr header.b="GMo5yCAo" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726006AbgDSJy5 (ORCPT ); Sun, 19 Apr 2020 05:54:57 -0400 Received: from pegase1.c-s.fr ([93.17.236.30]:22141 "EHLO pegase1.c-s.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725832AbgDSJy4 (ORCPT ); Sun, 19 Apr 2020 05:54:56 -0400 Received: from localhost (mailhub1-int [192.168.12.234]) by localhost (Postfix) with ESMTP id 494lYR3Xc3z9tyDn; Sun, 19 Apr 2020 11:54:51 +0200 (CEST) Authentication-Results: localhost; dkim=pass reason="1024-bit key; insecure key" header.d=c-s.fr header.i=@c-s.fr header.b=GMo5yCAo; dkim-adsp=pass; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at c-s.fr Received: from pegase1.c-s.fr ([192.168.12.234]) by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024) with ESMTP id nAxBIwYGtWFH; Sun, 19 Apr 2020 11:54:51 +0200 (CEST) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase1.c-s.fr (Postfix) with ESMTP id 494lYR2Jrbz9tyDm; Sun, 19 Apr 2020 11:54:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=c-s.fr; s=mail; t=1587290091; bh=SLDSEP4XrdF5pJ0VblHWXGRy2Y2Yp8jccdJto8CbGTc=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=GMo5yCAoAVsWdEb2nyZuGs/NcpmrR+c2viuYh+CDDCjDG1lIiDMq2G0pmSZt72ixe Nk4oU7rpwicZO+eB/vvr2dnukKPFj//Ec4hCZumhU5k0eGF6SaCEn/t//2hPYX1I9R 8JM1OIFgJImOH1tnT7v7AWgzpBxrn7PMltFf9/5U= Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 94BBE8B76F; Sun, 19 Apr 2020 11:54:54 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id IH0yUOqJJstY; Sun, 19 Apr 2020 11:54:54 +0200 (CEST) Received: from [192.168.4.90] (unknown [192.168.4.90]) by messagerie.si.c-s.fr (Postfix) with ESMTP id EADAD8B752; Sun, 19 Apr 2020 11:54:53 +0200 (CEST) Subject: Re: [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32 To: "Eric W. Biederman" , Christoph Hellwig Cc: Arnd Bergmann , x86@kernel.org, linux-kernel@vger.kernel.org, Alexander Viro , linux-fsdevel@vger.kernel.org, Andrew Morton , linuxppc-dev@lists.ozlabs.org, Jeremy Kerr References: <20200414070142.288696-1-hch@lst.de> <20200414070142.288696-3-hch@lst.de> <87pnc5akhk.fsf@x220.int.ebiederm.org> <87k12dakfx.fsf_-_@x220.int.ebiederm.org> <87v9lx3t4j.fsf@x220.int.ebiederm.org> From: Christophe Leroy Message-ID: Date: Sun, 19 Apr 2020 11:54:41 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <87v9lx3t4j.fsf@x220.int.ebiederm.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 18/04/2020 à 13:55, Eric W. Biederman a écrit : > Christophe Leroy writes: > >> 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. > > I don't follow. You don't like temporary structures in the coredump > code or temporary structures in copy_siginfo_to_user32? In copy_siginfo_to_user32() > > A temporary structure in copy_siginfo_to_user is pretty much required > so that it can be zeroed to guarantee we don't pass a structure with > holes to userspace. Why ? We can zeroize the user structure directly, either with clear_user() or with some not yet existing unsafe_clear_user() or equivalent. > > The implementation of copy_siginfo_to_user32 used to use the equivalent > of user_access_begin() and user_access_end() and the code was a mess > that was very difficult to reason about. I recall their being holes > in the structure that were being copied to userspace. > > Meanwhile if you are going to set all of the bytes a cache hot temporary > structure is quite cheap. But how can we be sure it is cache hot ? As we are using memset() to zeroize it, it won't be loaded from memory as it will use dcbz instruction, but at some point in time it will get flushed back to memory, that's consuming anyway. Unless we invalidate it after the copy, but that becomes complex. Christophe