linux-kernel - Re: [PATCH 4.4 05/48] mm: fix devm_memremap_pages crash, use mem_hotplug

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:   Thu, 09 Feb 2017 15:26:39 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     Dan Williams <dan.j.williams@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     stable@...r.kernel.org, Christoph Hellwig <hch@....de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 4.4 05/48] mm: fix devm_memremap_pages crash, use
 mem_hotplug_{begin, done}

On Wed, 2017-01-18 at 11:46 +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Dan Williams <dan.j.williams@...el.com>
> 
> commit f931ab479dd24cf7a2c6e2df19778406892591fb upstream.
> 
> Both arch_add_memory() and arch_remove_memory() expect a single threaded
> context.
[...]
> The result is that two threads calling devm_memremap_pages()
> simultaneously can end up colliding on pgd initialization.  This leads
> to crash signatures like the following where the loser of the race
> initializes the wrong pgd entry:
[...]
> Hold the standard memory hotplug mutex over calls to
> arch_{add,remove}_memory().
[...]

This is not a sufficient fix, because memory_hotplug.c still assumes
there's only one 'writer':

void put_online_mems(void)
{
	...
        if (!--mem_hotplug.refcount && unlikely(mem_hotplug.active_writer))
                wake_up_process(mem_hotplug.active_writer);
        ...
}

void mem_hotplug_begin(void)
{
        mem_hotplug.active_writer = current;

        memhp_lock_acquire();
        for (;;) {
                mutex_lock(&mem_hotplug.lock);
                if (likely(!mem_hotplug.refcount))
                        break;
                __set_current_state(TASK_UNINTERRUPTIBLE);
                mutex_unlock(&mem_hotplug.lock);
                schedule();
        }
}

With multiple writers, one or more of them may hang or
{get,put}_online_mems() may mess up the hotplug reference count.

Is there a good reason that memory_hotplug.c isn't using an rwsem?

Ben.

-- 
Ben Hutchings
All the simple programs have been written, and all the good names
taken.


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)