linux-kernel - Re: [PATCH v2 1/2] mm/madvise: introduce PR_MADV_SELF flag to process

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4740dfc7-71da-4eb4-b071-35116288571f@lucifer.local>
Date: Tue, 24 Sep 2024 14:12:49 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Pedro Falcato <pedro.falcato@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Suren Baghdasaryan <surenb@...gle.com>, Arnd Bergmann <arnd@...db.de>,
        Shakeel Butt <shakeel.butt@...ux.dev>, linux-api@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Minchan Kim <minchan@...nel.org>,
        Richard Henderson <richard.henderson@...aro.org>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>, linux-alpha@...r.kernel.org,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org,
        "James E . J . Bottomley" <James.Bottomley@...senpartnership.com>,
        Helge Deller <deller@....de>, linux-parisc@...r.kernel.org,
        Chris Zankel <chris@...kel.net>, Max Filippov <jcmvbkbc@...il.com>
Subject: Re: [PATCH v2 1/2] mm/madvise: introduce PR_MADV_SELF flag to
 process_madvise()

On Tue, Sep 24, 2024 at 01:51:11PM GMT, Pedro Falcato wrote:
> On Tue, Sep 24, 2024 at 12:16:27PM GMT, Lorenzo Stoakes wrote:
> > process_madvise() was conceived as a useful means for performing a vector
> > of madvise() operations on a remote process's address space.
> >
> > However it's useful to be able to do so on the current process also. It is
> > currently rather clunky to do this (requiring a pidfd to be opened for the
> > current process) and introduces unnecessary overhead in incrementing
> > reference counts for the task and mm.
> >
> > Avoid all of this by providing a PR_MADV_SELF flag, which causes
> > process_madvise() to simply ignore the pidfd parameter and instead apply
> > the operation to the current process.
> >
>
> How about simply defining a pseudo-fd PIDFD_SELF in the negative int space?
> There's precedent for it in the fs space (AT_FDCWD). I think it's more ergonomic
> and if you take out the errno space we have around 2^31 - 4096 available sentinel
> values.
>
> e.g:
>
> /* AT_FDCWD = -10, -1 is dangerous, pick a different value */
> #define PIDFD_SELF   -11
>
> int pidfd = target_pid == getpid() ? PIDFD_SELF : pidfd_open(...);
> process_madvise(pidfd, ...);
>
>
> What do you think?

I like the way you're thinking, but I don't think this is something we can
do in the context of this series.

I mean, I totally accept using a flag here and ignoring the pidfd field is
_ugly_, no question. But I'm trying to find the smallest change that
achieves what we want.

To add such a sentinel would be a change to the pidfd mechanism as a whole,
and we'd be left in the awkward situation that no other user of the pidfd
mechanism would be implementing this, but we'd have to expose this as a
general sentinel value for all pidfd users.

One nice thing with doing this as a flag is that, later, if somebody is
willing to do the larger task of having a special sentinel pidfd value to
mean 'the current process', we could use this in process_madvise() and
deprecate this flag :)

>
> --
> Pedro