linux-kernel - Re: [PATCH v3] mm/madvise: unrestrict 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <245f73e4-8076-4cdc-a3da-6c90d048dfc9@lucifer.local>
Date: Fri, 27 Sep 2024 09:21:03 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Christian Brauner <brauner@...nel.org>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>,
        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>,
        linux-api@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Minchan Kim <minchan@...nel.org>,
        pedro.falcato@...il.com
Subject: Re: [PATCH v3] mm/madvise: unrestrict process_madvise() for current
 process

On Fri, Sep 27, 2024 at 10:04:25AM GMT, Christian Brauner wrote:
> On Thu, Sep 26, 2024 at 08:52:32AM GMT, Shakeel Butt wrote:
> > On Thu, Sep 26, 2024 at 04:10:19PM GMT, Lorenzo Stoakes wrote:
> > > The process_madvise() call was introduced in commit ecb8ac8b1f14
> > > ("mm/madvise: introduce process_madvise() syscall: an external memory
> > > hinting API") as a means of performing madvise() operations on another
> > > process.
> > >
> > > However, as it provides the means by which to perform multiple madvise()
> > > operations in a batch via an iovec, it is useful to utilise the same
> > > interface for performing operations on the current process rather than a
> > > remote one.
> > >
> > > Commit 22af8caff7d1 ("mm/madvise: process_madvise() drop capability check
> > > if same mm") removed the need for a caller invoking process_madvise() on
> > > its own pidfd to possess the CAP_SYS_NICE capability, however this leaves
> > > the restrictions on operation in place.
> > >
> > > Resolve this by only applying the restriction on operations when accessing
> > > a remote process.
> > >
> > > Moving forward we plan to implement a simpler means of specifying this
> > > condition other than needing to establish a self pidfd, perhaps in the form
> > > of a sentinel pidfd.
> > >
> > > Also take the opportunity to refactor the system call implementation
> > > abstracting the vectorised operation.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> >
> > Acked-by: Shakeel Butt <shakeel.butt@...ux.dev>
> >
> > > ---
> > > v3:
> > > * Avoid introducing PR_MADV_SELF and defer a non-pidfd version until later.
> >
> > Seems like a good plan to decouple this patch from PR_MADV_SELF vs
> > PIDFD_SELF decision. I am hoping to see the follow up patch as well.
>
> PIDFD_SELF should absolutely not be a per-system call thing. It should
> be generic across all pidfd based system calls similar to AT_FDCWD.
>
> IOW, that should be in:
>
> include/uapi/linux/pidfd.h
>
> #define PIDFD_SELF -200

Yes this is what I was saying elsewhere in the thread :) this is why it's
important to have this as a separate enterprise.

And indeed this is the intent, I will be working on a separate patch series
to this effect. It also gives us the space to implement it in calls which
use pidfd where it makes sense and to extend testing accordingly.