linux-kernel - Re: Using page-fault handler in binder

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] [day] [month] [year] [list]

Message-ID: <ZEHbsOmokBGlzNjo@google.com>
Date:   Fri, 21 Apr 2023 00:41:20 +0000
From:   Carlos Llamas <cmllamas@...gle.com>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     linux-mm@...ck.org, tkjos@...gle.com, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: Using page-fault handler in binder

On Thu, Apr 20, 2023 at 02:35:00PM -0700, Suren Baghdasaryan wrote:
> On Thu, Apr 20, 2023 at 1:34 PM Carlos Llamas <cmllamas@...gle.com> wrote:
> >
> > Instead of caching page pointers, I believe binder could make use of
> > get_user_pages_remote() and trigger a page fault as needed. Basically,
> > implement something similar to access_remote_vm() but that instead does
> > a copy_from_user(). However, I don't see many in-tree users of these
> > routines, so I wonder if I'm overlooking something in this approach?
> 
> Sounds doable but there might be GUP details I'm missing...
> 
> >
> > One more reason for the page caching in binder is being able to free
> > pages from the shrinker's callback. Unused pages are added to an LRU
> > list and we manually zap/free them when they are reclaimed. It is not
> > evident to me how can binder mark a range within the vma region as
> > reclaimable?
> 
> I think you would have to release the individual pages using
> put_page(). Also note that get_user_pages_remote() is being
> deprecated, see:
> https://elixir.bootlin.com/linux/latest/source/mm/gup.c#L2171

Yeah, I remember reading that comment but it doesn't make sense since
the suggested alternatives do not support operations on _remote_ mm.

I actually tracked this down and it seems the original comment was made
for get_user_pages() instead, in commit f0818f472d8d ("mm: gup: add
get_user_pages_locked and get_user_pages_unlocked"). The whole comment
block was then carried over without update into get_user_pages_remote()
in commit 1e9877902dc7 ("mm/gup: Introduce get_user_pages_remote()").
The comment block was finally switched to use the "_remote" portion in
commit adc8cb406e52 ("mm/gup.c: update the documentation"). In reality,
this comment should be relocated to the get_user_pages() section.

> 
> >
> > Any pointers are appreciated!
> >
> > --
> > Carlos Llamas