* [PATCH] mm/madvise: process_madvise() drop capability check if same mm
@ 2024-09-13 14:06 Lorenzo Stoakes
2024-09-13 14:31 ` Liam R. Howlett
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Lorenzo Stoakes @ 2024-09-13 14:06 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Vlastimil Babka, Liam Howlett,
Shakeel Butt, Suren Baghdasaryan
In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
process_madvise") process_madvise() was updated to require the caller to
possess the CAP_SYS_NICE capability to perform the operation, in addition
to a check against PTRACE_MODE_READ performed by mm_access().
The mm_access() function explicitly checks to see if the address space of
the process being referenced is the current one, in which case no check is
performed.
We, however, do not do this when checking the CAP_SYS_NICE capability. This
means that we insist on the caller possessing this capability in order to
perform madvise() operations on its own address space, which seems
nonsensical.
Simply add a check to allow for an invocation of this function with pidfd
set to the current process without elevation.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
mm/madvise.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/madvise.c b/mm/madvise.c
index 4e64770be16c..ff139e57cca2 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1520,7 +1520,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
* Require CAP_SYS_NICE for influencing process performance. Note that
* only non-destructive hints are currently supported.
*/
- if (!capable(CAP_SYS_NICE)) {
+ if (mm != current->mm && !capable(CAP_SYS_NICE)) {
ret = -EPERM;
goto release_mm;
}
--
2.46.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/madvise: process_madvise() drop capability check if same mm
2024-09-13 14:06 [PATCH] mm/madvise: process_madvise() drop capability check if same mm Lorenzo Stoakes
@ 2024-09-13 14:31 ` Liam R. Howlett
2024-09-13 14:35 ` Liam R. Howlett
2024-09-13 14:51 ` Vlastimil Babka
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Liam R. Howlett @ 2024-09-13 14:31 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, linux-mm, linux-kernel, Vlastimil Babka,
Shakeel Butt, Suren Baghdasaryan
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [240913 10:06]:
> In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
> process_madvise") process_madvise() was updated to require the caller to
> possess the CAP_SYS_NICE capability to perform the operation, in addition
> to a check against PTRACE_MODE_READ performed by mm_access().
>
> The mm_access() function explicitly checks to see if the address space of
> the process being referenced is the current one, in which case no check is
> performed.
>
> We, however, do not do this when checking the CAP_SYS_NICE capability. This
> means that we insist on the caller possessing this capability in order to
> perform madvise() operations on its own address space, which seems
> nonsensical.
>
> Simply add a check to allow for an invocation of this function with pidfd
> set to the current process without elevation.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Probably needs a fixes 96cfe2c0fd23 tag?
Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> ---
> mm/madvise.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 4e64770be16c..ff139e57cca2 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1520,7 +1520,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
> * Require CAP_SYS_NICE for influencing process performance. Note that
> * only non-destructive hints are currently supported.
> */
> - if (!capable(CAP_SYS_NICE)) {
> + if (mm != current->mm && !capable(CAP_SYS_NICE)) {
> ret = -EPERM;
> goto release_mm;
> }
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/madvise: process_madvise() drop capability check if same mm
2024-09-13 14:31 ` Liam R. Howlett
@ 2024-09-13 14:35 ` Liam R. Howlett
0 siblings, 0 replies; 6+ messages in thread
From: Liam R. Howlett @ 2024-09-13 14:35 UTC (permalink / raw)
To: Shakeel Butt
Cc: Lorenzo Stoakes, Andrew Morton, linux-mm, linux-kernel,
Vlastimil Babka, Suren Baghdasaryan
..Add Shakeel's new email address
* Liam R. Howlett <Liam.Howlett@oracle.com> [240913 10:31]:
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [240913 10:06]:
> > In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
> > process_madvise") process_madvise() was updated to require the caller to
> > possess the CAP_SYS_NICE capability to perform the operation, in addition
> > to a check against PTRACE_MODE_READ performed by mm_access().
> >
> > The mm_access() function explicitly checks to see if the address space of
> > the process being referenced is the current one, in which case no check is
> > performed.
> >
> > We, however, do not do this when checking the CAP_SYS_NICE capability. This
> > means that we insist on the caller possessing this capability in order to
> > perform madvise() operations on its own address space, which seems
> > nonsensical.
> >
> > Simply add a check to allow for an invocation of this function with pidfd
> > set to the current process without elevation.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Probably needs a fixes 96cfe2c0fd23 tag?
>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
>
> > ---
> > mm/madvise.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 4e64770be16c..ff139e57cca2 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -1520,7 +1520,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
> > * Require CAP_SYS_NICE for influencing process performance. Note that
> > * only non-destructive hints are currently supported.
> > */
> > - if (!capable(CAP_SYS_NICE)) {
> > + if (mm != current->mm && !capable(CAP_SYS_NICE)) {
> > ret = -EPERM;
> > goto release_mm;
> > }
> > --
> > 2.46.0
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/madvise: process_madvise() drop capability check if same mm
2024-09-13 14:06 [PATCH] mm/madvise: process_madvise() drop capability check if same mm Lorenzo Stoakes
2024-09-13 14:31 ` Liam R. Howlett
@ 2024-09-13 14:51 ` Vlastimil Babka
2024-09-13 15:56 ` Shakeel Butt
2024-09-15 7:50 ` David Rientjes
3 siblings, 0 replies; 6+ messages in thread
From: Vlastimil Babka @ 2024-09-13 14:51 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: linux-mm, linux-kernel, Liam Howlett, Shakeel Butt, Suren Baghdasaryan
On 9/13/24 16:06, Lorenzo Stoakes wrote:
> In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
> process_madvise") process_madvise() was updated to require the caller to
> possess the CAP_SYS_NICE capability to perform the operation, in addition
> to a check against PTRACE_MODE_READ performed by mm_access().
>
> The mm_access() function explicitly checks to see if the address space of
> the process being referenced is the current one, in which case no check is
> performed.
>
> We, however, do not do this when checking the CAP_SYS_NICE capability. This
> means that we insist on the caller possessing this capability in order to
> perform madvise() operations on its own address space, which seems
> nonsensical.
>
> Simply add a check to allow for an invocation of this function with pidfd
> set to the current process without elevation.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/madvise.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 4e64770be16c..ff139e57cca2 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1520,7 +1520,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
> * Require CAP_SYS_NICE for influencing process performance. Note that
> * only non-destructive hints are currently supported.
> */
> - if (!capable(CAP_SYS_NICE)) {
> + if (mm != current->mm && !capable(CAP_SYS_NICE)) {
> ret = -EPERM;
> goto release_mm;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/madvise: process_madvise() drop capability check if same mm
2024-09-13 14:06 [PATCH] mm/madvise: process_madvise() drop capability check if same mm Lorenzo Stoakes
2024-09-13 14:31 ` Liam R. Howlett
2024-09-13 14:51 ` Vlastimil Babka
@ 2024-09-13 15:56 ` Shakeel Butt
2024-09-15 7:50 ` David Rientjes
3 siblings, 0 replies; 6+ messages in thread
From: Shakeel Butt @ 2024-09-13 15:56 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, linux-mm, linux-kernel, Vlastimil Babka,
Liam Howlett, Shakeel Butt, Suren Baghdasaryan
On Fri, Sep 13, 2024 at 03:06:28PM GMT, Lorenzo Stoakes wrote:
> In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
> process_madvise") process_madvise() was updated to require the caller to
> possess the CAP_SYS_NICE capability to perform the operation, in addition
> to a check against PTRACE_MODE_READ performed by mm_access().
>
> The mm_access() function explicitly checks to see if the address space of
> the process being referenced is the current one, in which case no check is
> performed.
>
> We, however, do not do this when checking the CAP_SYS_NICE capability. This
> means that we insist on the caller possessing this capability in order to
> perform madvise() operations on its own address space, which seems
> nonsensical.
>
> Simply add a check to allow for an invocation of this function with pidfd
> set to the current process without elevation.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/madvise: process_madvise() drop capability check if same mm
2024-09-13 14:06 [PATCH] mm/madvise: process_madvise() drop capability check if same mm Lorenzo Stoakes
` (2 preceding siblings ...)
2024-09-13 15:56 ` Shakeel Butt
@ 2024-09-15 7:50 ` David Rientjes
3 siblings, 0 replies; 6+ messages in thread
From: David Rientjes @ 2024-09-15 7:50 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, linux-mm, linux-kernel, Vlastimil Babka,
Liam Howlett, Shakeel Butt, Suren Baghdasaryan
On Fri, 13 Sep 2024, Lorenzo Stoakes wrote:
> In commit 96cfe2c0fd23 ("mm/madvise: replace ptrace attach requirement for
> process_madvise") process_madvise() was updated to require the caller to
> possess the CAP_SYS_NICE capability to perform the operation, in addition
> to a check against PTRACE_MODE_READ performed by mm_access().
>
> The mm_access() function explicitly checks to see if the address space of
> the process being referenced is the current one, in which case no check is
> performed.
>
> We, however, do not do this when checking the CAP_SYS_NICE capability. This
> means that we insist on the caller possessing this capability in order to
> perform madvise() operations on its own address space, which seems
> nonsensical.
>
> Simply add a check to allow for an invocation of this function with pidfd
> set to the current process without elevation.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Acked-by: David Rientjes <rientjes@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-15 7:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-13 14:06 [PATCH] mm/madvise: process_madvise() drop capability check if same mm Lorenzo Stoakes
2024-09-13 14:31 ` Liam R. Howlett
2024-09-13 14:35 ` Liam R. Howlett
2024-09-13 14:51 ` Vlastimil Babka
2024-09-13 15:56 ` Shakeel Butt
2024-09-15 7:50 ` David Rientjes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox