linux-kernel - Re: [RFC PATCH] mm/mbind: Introduce process_mbind() syscall for external memory binding

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]

Message-ID: <CACSyD1MrCzyV-93Ov07NpV3Nm3u0fYExmD1ShE_e2tapW6a6HA@mail.gmail.com>
Date:   Thu, 30 Nov 2023 17:34:04 +0800
From:   Zhongkun He <hezhongkun.hzk@...edance.com>
To:     Gregory Price <gregory.price@...verge.com>
Cc:     Vinicius Petrucci <vpetrucci@...il.com>, akpm@...ux-foundation.org,
        linux-mm@...r.kernel.org, linux-cxl@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-api@...r.kernel.org, minchan@...nel.org,
        dave.hansen@...ux.intel.com, x86@...nel.org,
        Jonathan.Cameron@...wei.com, aneesh.kumar@...ux.ibm.com,
        ying.huang@...el.com, dan.j.williams@...el.com, fvdl@...gle.com,
        surenb@...gle.com, rientjes@...gle.com, hannes@...xchg.org,
        mhocko@...e.com, Hasan.Maruf@....com, jgroves@...ron.com,
        ravis.opensrc@...ron.com, sthanneeru@...ron.com,
        emirakhur@...ron.com, vtavarespetr@...ron.com
Subject: Re: [RFC PATCH] mm/mbind: Introduce process_mbind() syscall for
 external memory binding

Hi Gregory, sorry for the late reply.

I tried pidfd_set_mempolicy(suggested by michal) about a year ago.
There is a problem here that may need attention.

A mempolicy can be either associated with a process or with a VMA.
All vma manipulation is somewhat protected by a down_read on
mmap_lock.In process context(in alloc_pages()) there is no locking
because only the process accesses its own state.

Now  we need to change the process context mempolicy specified
in pidfd. the mempolicy may about to be freed by
pidfd_set_mempolicy() while alloc_pages() is using it,
The race condition appears.

Say something like the following：

pidfd_set_mempolicy()        target task stack:
                                               alloc_pages:
                                             mpol = p->mempolicy;
  task_lock(task);
  old = task->mempolicy;
  task->mempolicy = new;
  task_unlock(task);
  mpol_put(old);
                                           /*old mpol has been freed.*/
                                           policy_node(...., mpol)
                                          __alloc_pages();

To reduce the use of locks and atomic operations(mpol_get/put)
in the hot path, there are no references or lock protections here
for task mempolicy.

It would be great if your refactoring has a good solution.

Thanks.

On Sat, Nov 25, 2023 at 4:09 AM Gregory Price
<gregory.price@...verge.com> wrote:
>
> On Fri, Nov 24, 2023 at 04:13:41PM +0800, Zhongkun He wrote:
> >
> > Per my understanding,  the process_mbind() is implementable without
> > many difficult challenges，
> > since it is always protected by mm->mmap_lock. But task mempolicy does
> > not acquire any lock
> > in alloc_pages().
>
> per-vma policies are protected by the mmap lock, while the task
> mempolicy is protected by the task lock on replacement, and
> task->mems_allowed (protected by task_lock).
>
> There is an update in my refactor tickets that enforces the acquisition
> of task_lock on mpol_set_nodemask, which prevents the need for
> alloc_pages to do anything else.  That's not present in this patch.
>
> Basically mems_allowed deals with the majority of situations, and
> mmap_lock deals with per-vma mempolicy changes and migrations.
>
> ~Gregory