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
In-Reply-To: <41ab11ba9fa03940d129f478109d048b197d9a59.camel@intel.com>
References: <1536723394-14616-1-git-send-email-bin.yang@intel.com>
 <CAGXu5j+C9pApyP4ab6usOudFFFBqX5WpZQYaH6m2BpmABB2W6w@mail.gmail.com> <41ab11ba9fa03940d129f478109d048b197d9a59.camel@intel.com>
From:   Kees Cook <keescook@chromium.org>
Date:   Wed, 12 Sep 2018 21:25:05 -0700
Message-ID: <CAGXu5jJigfeAgzoHRR-sny651kcB5fQEGF_X5n+Wi02EfwP9iA@mail.gmail.com>
Subject: Re: [PATCH] pstore: fix incorrect persistent ram buffer mapping
To:     "Yang, Bin" <bin.yang@intel.com>
Cc:     "ccross@android.com" <ccross@android.com>,
        "Luck, Tony" <tony.luck@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "anton@enomsg.org" <anton@enomsg.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Sep 12, 2018 at 6:21 PM, Yang, Bin <bin.yang@intel.com> wrote:
> On Wed, 2018-09-12 at 10:44 -0700, Kees Cook wrote:
>> On Tue, Sep 11, 2018 at 8:36 PM, Bin Yang <bin.yang@intel.com> wrote:
>> > persistent_ram_vmap() returns the page start vaddr.
>> > persistent_ram_iomap() supports non-page-aligned mapping.
>>
>> Oh, yes, good catch. This should probably be explicitly mentioned in
>> comments for these functions.
>>
>> > persistent_ram_buffer_map() always adds offset-in-page to the vaddr
>> > returned from these two functions, which causes incorrect mapping of
>> > non-page-aligned persistent ram buffer.
>>
>> How did you find this problem, and/or how was the problem manifesting?
>
> By default, ftrace_size is 4096 and max_ftrace_cnt is nr_cpu_ids. The
> zone_sz in ramoops_init_przs() is 4096/nr_cpu_ids which might not be
> page aligned. If the offset-in-page > 2048, the vaddr will be in next
> page. If the next page is not mapped, it will cause kernel panic.
>
> I just wanted to enable this driver on my board and did not change the
> default value of ftrace_size. It resulted kernel panic as below:
>
>
> [    0.074231] BUG: unable to handle kernel paging request at
> ffffa19e0081b000

Perfect, thanks! I've updated your commit log to include these details
now. Should be in linux-next shortly.

-Kees

-- 
Kees Cook
Pixel Security