On 16.11.22 19:16, Linus Torvalds wrote:
On Wed, Nov 16, 2022 at 2:30 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
Let's make it clearer that functionality provided by FOLL_FORCE is
really only for ptrace access.
I'm not super-happy about this one.
I do understand the "let's rename the bit so that no new user shows up".
And it's true that the main traditional use is ptrace.
But from the patch itself it becomes obvious that no, it's not *just*
ptrace. At least not yet.
It's used for get_arg_page(), which uses it to basically look up (and
install) pages in the newly created VM.
Now, I'm not entirely sure why it even uses FOLL_FORCE, - I think it
might be historical, because the target should always be the new stack
vma.
Following the history of it is a big of a mess, because there's a
number of renamings and re-organizations, but it seems to go back to
2007 and commit b6a2fea39318 ("mm: variable length argument support").
Right.
Before that commit, we kept our own array of "this is the set of pages
that I will install in the new VM". That commit basically just inserts
the pages directly into the VM instead, getting rid of the array size
limitation.
So at a minimum, I think that FOLL_FORCE would need to be removed
before any renaming to FOLL_PTRACE, because that's not some kind of
small random case.
It *might* be as simple as just removing it, but maybe there's some
reason for having it that I don't immediately see.
Right, I have the same feeling. It might just be a copy-and-paste legacy
leftover.
There _are_ also small random cases too, like get_cmdline(). Maybe
that counts as ptrace, but the execve() case most definitely does not.
I agree. I'd suggest moving forward without this (last) patch for now
and figuring out how to further cleanup FOLL_FORCE usage on top.
@Andrew, if you intend to put this into mm-unstable, please drop the
last patch for now.
--
Thanks,
David / dhildenb
