Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751503AbdCDGF7 (ORCPT <rfc822;w@1wt.eu>);
        Sat, 4 Mar 2017 01:05:59 -0500
Received: from mail-pf0-f193.google.com ([209.85.192.193]:36437 "EHLO
        mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750721AbdCDGF6 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sat, 4 Mar 2017 01:05:58 -0500
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC] arm64: support HAVE_ARCH_RARE_WRITE
From: Hoeun Ryu <hoeun.ryu@gmail.com>
X-Mailer: iPhone Mail (14D27)
In-Reply-To: <CALCETrVNRbswNWeoVLdntdNXmb4ur3a-xiVNqALajHNG-bLJgw@mail.gmail.com>
Date: Sat, 4 Mar 2017 14:56:44 +0900
Cc: "kernel-hardening@lists.openwall.com" 
        <kernel-hardening@lists.openwall.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Kees Cook <keescook@chromium.org>, Mark Rutland <mark.rutland@arm.com>,
        Andy Lutomirski <luto@kernel.org>, Emese Revfy <re.emese@gmail.com>,
        Russell King <linux@armlinux.org.uk>, PaX Team <pageexec@freemail.hu>,
        X86 ML <x86@kernel.org>
Message-Id: <E951BDD3-A4C9-436B-B886-2E6BF0A16246@gmail.com>
References: <1488466831-13918-1-git-send-email-hoeun.ryu@gmail.com> <CALCETrVNRbswNWeoVLdntdNXmb4ur3a-xiVNqALajHNG-bLJgw@mail.gmail.com>
To: Andy Lutomirski <luto@amacapital.net>
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2466Dlv009055
Content-Length: 1498
Lines: 51


> On Mar 4, 2017, at 5:50 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> 
>> On Thu, Mar 2, 2017 at 7:00 AM, Hoeun Ryu <hoeun.ryu@gmail.com> wrote:
>> +unsigned long __rare_write_rw_alias_start = TASK_SIZE_64 / 4;
>> +
>> +__always_inline unsigned long __arch_rare_write_map(void)
>> +{
>> +       struct mm_struct *mm = &rare_write_mm;
>> +
>> +       preempt_disable();
>> +
>> +       __switch_mm(mm);
> 
> ...
> 
>> +__always_inline unsigned long __arch_rare_write_unmap(void)
>> +{
>> +       struct mm_struct *mm = current->active_mm;
>> +
>> +       __switch_mm(mm);
>> +
> 
> This reminds me: this code imposes constraints on the context in which
> it's called.  I'd advise making it very explicit, asserting
> correctness, and putting the onus on the caller to set things up.  For
> example:
> 
> DEBUG_LOCKS_WARN_ON(preemptible() || in_interrupt() || in_nmi());
> 

OK. I will add some onus in the next version.

> in both the map and unmap functions, along with getting rid of the
> preempt_disable().  I don't think we want the preempt-disabledness to
> depend on the arch.  The generic non-arch rare_write helpers can do
> the preempt_disable().
> 

I think I can fix this in the next version when Kees send the next version of RFC.

> This code also won't work if the mm is wacky when called.  On x86, we could do:
> 
> DEBUG_LOCKS_WARN_ON(read_cr3() != current->active_mm->pgd);
> 
> or similar (since that surely doesn't compile as is).
> 
> --Andy

Thank you for the review.