linux-kernel - Re: [PATCH] memremap: Fix NULL pointer BUG in get_zone_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Aug 2016 07:55:21 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     "Kani, Toshimitsu" <toshi.kani@....com>
Cc:     "Mulumudi, Abhilash Kumar" <m.abhilash-kumar@....com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        "ard.biesheuvel@...aro.org" <ard.biesheuvel@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "brian.starkey@....com" <brian.starkey@....com>
Subject: Re: [PATCH] memremap: Fix NULL pointer BUG in get_zone_device_page()

[ adding Konstantin ]

On Wed, Aug 24, 2016 at 7:40 AM, Kani, Toshimitsu <toshi.kani@....com> wrote:
> On Tue, 2016-08-23 at 21:48 -0700, Dan Williams wrote:
>> On Tue, Aug 23, 2016 at 9:28 PM, Kani, Toshimitsu <toshi.kani@....com
>> >
>> > BTW, why does devm_memremap_pages() put a whole range to
>> > pgmap_radix as device memory, but only initialize page->pgmap for
>> > its data range?  Is there particular reason for this inconsistency?
>>
>> The radix tree is indexed by section number, but we don't always
>> initialize a full section.  The cases when we don't use a full
>> section is when it overlaps device metadata, or if a platform
>> multiplexes the device memory range with another resource within the
>> same section.
>
> I see, but I still feel odd about making get_dev_pagemap() to work for
> metadata, but get_page() -> get_zone_device_page() to crash like this.
>  follow_devmap_pmd() assumes get_page() to work when get_dev_pagemap()
> returns a valid pgmap...
>

We could leave the unmapped portions of a section out of the radix,
but I'm also worried about keeping the get_dev_pagemap() lookup cheap.
I saw that Konstantin has some proposed changes to the radix api to
make it easier to fill a range [1].  I might switch to
radix_tree_fill_range() when it becomes available.

[1]: https://github.com/koct9i/linux/commits/radix-tree