process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted hunk
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
quoted
+ */
+ if (!capable(CAP_SYS_NICE)) {
+ ret = -EPERM;
+ goto release_mm;
+ }
+
total_len = iov_iter_count(&iter);
while (iov_iter_count(&iter)) {
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
On Wed, Mar 3, 2021 at 4:04 PM Shakeel Butt [off-list ref] wrote:
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
I think so. Destructive hints affect the data, so we will probably
need stricter checks for those hints.
From: David Hildenbrand <hidden> Date: 2021-03-05 17:38:59
On 04.03.21 01:03, Shakeel Butt wrote:
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
--
Thanks,
David / dhildenb
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
From: David Hildenbrand <hidden> Date: 2021-03-05 17:54:14
On 05.03.21 18:45, Shakeel Butt wrote:
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
quoted
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
Thanks, somehow I missed that (not that it really changed my opinion on
the approach while skimming over the discussion :) will have a more
detailed look)
--
Thanks,
David / dhildenb
On Fri, Mar 5, 2021 at 9:52 AM David Hildenbrand [off-list ref] wrote:
On 05.03.21 18:45, Shakeel Butt wrote:
quoted
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
quoted
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
Thanks, somehow I missed that (not that it really changed my opinion on
the approach while skimming over the discussion :) will have a more
detailed look)
The latest version of that patchset is:
https://lore.kernel.org/patchwork/patch/1344419/
Yeah, memory reaping is a special case when we are operating on a
dying process to speed up the release of its memory. I don't know if
for that particular case we need to make the checks stricter. It's a
dying process anyway and the data is being destroyed. Allowing to
speed up that process probably can still use CAP_SYS_NICE.
From: David Hildenbrand <hidden> Date: 2021-03-05 18:24:15
On 05.03.21 19:08, Suren Baghdasaryan wrote:
On Fri, Mar 5, 2021 at 9:52 AM David Hildenbrand [off-list ref] wrote:
quoted
On 05.03.21 18:45, Shakeel Butt wrote:
quoted
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
quoted
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
Thanks, somehow I missed that (not that it really changed my opinion on
the approach while skimming over the discussion :) will have a more
detailed look)
The latest version of that patchset is:
https://lore.kernel.org/patchwork/patch/1344419/
Yeah, memory reaping is a special case when we are operating on a
dying process to speed up the release of its memory. I don't know if
for that particular case we need to make the checks stricter. It's a
dying process anyway and the data is being destroyed. Allowing to
speed up that process probably can still use CAP_SYS_NICE.
I know, unrelated discussion (sorry, I don't have above thread in my
archive anymore due to automatic cleanups ...) , but introducing
MADV_DONTEED on a remote processes, having to tweak range logic because
we always want to apply it to the whole MM, just to speed up memory
reaping sounds like completely abusing madvise()/process_madvise() to me.
You want different semantics than MADV_DONTNEED. You want different
semantics than madvise.
Simple example: mlock()ed pages in the target process. MADV_DONTNEED
would choke on that. For the use case of reaping, you certainly don't care.
I am not sure if process_madvise() is the right interface to enforce
discarding of all target memory.
Not to mention that MADV_FREE doesn't make any sense IMHO for memory
reaping ... no to mention exposing this via process_madvise().
--
Thanks,
David / dhildenb
On Fri, Mar 5, 2021 at 10:23 AM David Hildenbrand [off-list ref] wrote:
On 05.03.21 19:08, Suren Baghdasaryan wrote:
quoted
On Fri, Mar 5, 2021 at 9:52 AM David Hildenbrand [off-list ref] wrote:
quoted
On 05.03.21 18:45, Shakeel Butt wrote:
quoted
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
quoted
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
Thanks, somehow I missed that (not that it really changed my opinion on
the approach while skimming over the discussion :) will have a more
detailed look)
The latest version of that patchset is:
https://lore.kernel.org/patchwork/patch/1344419/
Yeah, memory reaping is a special case when we are operating on a
dying process to speed up the release of its memory. I don't know if
for that particular case we need to make the checks stricter. It's a
dying process anyway and the data is being destroyed. Allowing to
speed up that process probably can still use CAP_SYS_NICE.
I know, unrelated discussion (sorry, I don't have above thread in my
archive anymore due to automatic cleanups ...) , but introducing
MADV_DONTEED on a remote processes, having to tweak range logic because
we always want to apply it to the whole MM, just to speed up memory
reaping sounds like completely abusing madvise()/process_madvise() to me.
You want different semantics than MADV_DONTNEED. You want different
semantics than madvise.
Simple example: mlock()ed pages in the target process. MADV_DONTNEED
would choke on that. For the use case of reaping, you certainly don't care.
I am not sure if process_madvise() is the right interface to enforce
discarding of all target memory.
Not to mention that MADV_FREE doesn't make any sense IMHO for memory
reaping ... no to mention exposing this via process_madvise().
From: David Hildenbrand <hidden> Date: 2021-03-05 19:41:51
Am 05.03.2021 um 19:36 schrieb Suren Baghdasaryan [off-list ref]:
On Fri, Mar 5, 2021 at 10:23 AM David Hildenbrand [off-list ref] wrote:
quoted
quoted
On 05.03.21 19:08, Suren Baghdasaryan wrote:
On Fri, Mar 5, 2021 at 9:52 AM David Hildenbrand [off-list ref] wrote:
quoted
On 05.03.21 18:45, Shakeel Butt wrote:
quoted
On Fri, Mar 5, 2021 at 9:37 AM David Hildenbrand [off-list ref] wrote:
quoted
On 04.03.21 01:03, Shakeel Butt wrote:
quoted
On Wed, Mar 3, 2021 at 3:34 PM Suren Baghdasaryan [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 3:17 PM Shakeel Butt [off-list ref] wrote:
quoted
On Wed, Mar 3, 2021 at 10:58 AM Suren Baghdasaryan [off-list ref] wrote:
quoted
process_madvise currently requires ptrace attach capability.
PTRACE_MODE_ATTACH gives one process complete control over another
process. It effectively removes the security boundary between the
two processes (in one direction). Granting ptrace attach capability
even to a system process is considered dangerous since it creates an
attack surface. This severely limits the usage of this API.
The operations process_madvise can perform do not affect the correctness
of the operation of the target process; they only affect where the data
is physically located (and therefore, how fast it can be accessed).
What we want is the ability for one process to influence another process
in order to optimize performance across the entire system while leaving
the security boundary intact.
Replace PTRACE_MODE_ATTACH with a combination of PTRACE_MODE_READ
and CAP_SYS_NICE. PTRACE_MODE_READ to prevent leaking ASLR metadata
and CAP_SYS_NICE for influencing process performance.
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Kees Cook <redacted>
Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: David Rientjes <rientjes@google.com>
---
changes in v3
- Added Reviewed-by: Kees Cook [off-list ref]
- Created man page for process_madvise per Andrew's request: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=a144f458bad476a3358e3a45023789cb7bb9f993
- cc'ed stable@vger.kernel.org # 5.10+ per Andrew's request
- cc'ed linux-security-module@vger.kernel.org per James Morris's request
mm/madvise.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
How is non-destructive defined? Is MADV_DONTNEED non-destructive?
Non-destructive in this context means the data is not lost and can be
recovered. I follow the logic described in
https://lwn.net/Articles/794704/ where Minchan was introducing
MADV_COLD and MADV_PAGEOUT as non-destructive versions of MADV_FREE
and MADV_DONTNEED. Following that logic, MADV_FREE and MADV_DONTNEED
would be considered destructive hints.
Note that process_madvise_behavior_valid() allows only MADV_COLD and
MADV_PAGEOUT at the moment, which are both non-destructive.
There is a plan to support MADV_DONTNEED for this syscall. Do we need
to change these access checks again with that support?
Eh, I absolutely don't think letting another process discard memory in
another process' address space is a good idea. The target process can
observe that easily and might even run into real issues.
What's the use case?
Thanks, somehow I missed that (not that it really changed my opinion on
the approach while skimming over the discussion :) will have a more
detailed look)
The latest version of that patchset is:
https://lore.kernel.org/patchwork/patch/1344419/
Yeah, memory reaping is a special case when we are operating on a
dying process to speed up the release of its memory. I don't know if
for that particular case we need to make the checks stricter. It's a
dying process anyway and the data is being destroyed. Allowing to
speed up that process probably can still use CAP_SYS_NICE.
I know, unrelated discussion (sorry, I don't have above thread in my
archive anymore due to automatic cleanups ...) , but introducing
MADV_DONTEED on a remote processes, having to tweak range logic because
we always want to apply it to the whole MM, just to speed up memory
reaping sounds like completely abusing madvise()/process_madvise() to me.
You want different semantics than MADV_DONTNEED. You want different
semantics than madvise.
Simple example: mlock()ed pages in the target process. MADV_DONTNEED
would choke on that. For the use case of reaping, you certainly don't care.
I am not sure if process_madvise() is the right interface to enforce
discarding of all target memory.
Not to mention that MADV_FREE doesn't make any sense IMHO for memory
reaping ... no to mention exposing this via process_madvise().