Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180907194852.3C351B82@viggo.jf.intel.com> <20180907194900.DF3B41C0@viggo.jf.intel.com>
In-Reply-To: <20180907194900.DF3B41C0@viggo.jf.intel.com>
From:   Jann Horn <jannh@google.com>
Date:   Sat, 8 Sep 2018 02:37:27 +0200
Message-ID: <CAG48ez1wuPaYmjNpuhA9pUhg+4Eu33Kavtmd9Ew8qq-NDHrwzw@mail.gmail.com>
Subject: Re: [RFC][PATCH 5/8] x86/mm: fix exception table comments
To:     Dave Hansen <dave.hansen@linux.intel.com>
Cc:     kernel list <linux-kernel@vger.kernel.org>,
        sean.j.christopherson@intel.com,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Andy Lutomirski <luto@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Sep 8, 2018 at 2:22 AM Dave Hansen <dave.hansen@linux.intel.com> wrote:
> +        * Kernel-mode access to the user address space should only occur
> +        * inside well-defined areas of code listed in the exception

Actually, not areas, but single whitelisted instructions. It would
probably be nice to say that more clearly.

From arch/x86/include/asm/extable.h:

/*
 * The exception table consists of triples of addresses relative to the
 * exception table entry itself. The first address is of an instruction
 * that is allowed to fault, the second is the target at which the program
 * should continue. The third is a handler function to deal with the fault
 * caused by the instruction in the first field.
 *
 * All the routines below use bits of fixup code that are out of line
 * with the main instruction path.  This means when everything is well,
 * we don't even have to jump over them.  Further, they do not intrude
 * on our cache or tlb entries.
 */

struct exception_table_entry {
        int insn, fixup, handler;
};

> +        * tables.  But, an erroneous kernel fault occurring outside one of
> +        * those areas which also holds mmap_sem might deadlock attempting
> +        * to validate the fault against the address space.
>          *
> -        * As the vast majority of faults will be valid we will only perform
> -        * the source reference check when there is a possibility of a
> -        * deadlock. Attempt to lock the address space, if we cannot we then
> -        * validate the source. If this is invalid we can skip the address
> -        * space check, thus avoiding the deadlock:
> +        * Only do the expensive exception table search when we might be at
> +        * risk of a deadlock:
> +        * 1. We failed to acquire mmap_sem, and
> +        * 2. The access was an explicit kernel-mode access
> +        *    (X86_PF_USER=0).
>          */
>         if (unlikely(!down_read_trylock(&mm->mmap_sem))) {
>                 if (!(sw_error_code & X86_PF_USER) &&
>                     !search_exception_tables(regs->ip)) {
> +                       /*
> +                        * Fault from code in kernel from
> +                        * which we do not expect faults.
> +                        */
>                         bad_area_nosemaphore(regs, sw_error_code, address, NULL);
>                         return;
>                 }
> _
>