On Mon, Dec 07, 2020 at 06:22:00PM -0800, Andrew Morton wrote: > On Mon, 7 Dec 2020 10:48:18 +0100 Oscar Salvador <osalvador@xxxxxxx> wrote: > > > madvise_inject_error() uses get_user_pages_fast to translate the > > address we specified to a page. > > After [1], we drop the extra reference count for memory_failure() path. > > That commit says that memory_failure wanted to keep the pin in order > > to take the page out of circulation. > > > > The truth is that we need to keep the page pinned, otherwise the > > page might be re-used after the put_page() and we can end up messing > > with someone else's memory. > > > > E.g: > > > > CPU0 > > process X CPU1 > > madvise_inject_error > > get_user_pages > > put_page > > page gets reclaimed > > process Y allocates the page > > memory_failure > > // We mess with process Y memory > > > > madvise() is meant to operate on a self address space, so messing with > > pages that do not belong to us seems the wrong thing to do. > > To avoid that, let us keep the page pinned for memory_failure as well. > > > > Pages for DAX mappings will release this extra refcount in > > memory_failure_dev_pagemap. > > Does the bug have any known user-visible effects? Is a deliberate > exploit conceivable? > > IOW, cc:stable and if so, why? This interface is a testing feature and only available only for privileged (CAP_SYS_ADMIN) users, so I don't think that this bug is critical. But if someone think it need to go to stable, I'm fine with that. Thanks, Naoya Horiguchi
