* [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: amd-gfx, dri-devel, linux-kernel, linux-fsdevel, linux-mm,
linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, peterz, acme, Kefeng Wang
Add vma_is_initial_stack() and vma_is_initial_heap() helper and use
them to simplify code.
v2:
- add comment for heap helper and remove one more goto cpy_name,
per David Hildenbrand
- add RB
v2:
- address comments per David Hildenbrand and Christian Göttsche
- fix selinux build
Kefeng Wang (4):
mm: factor out VMA stack and heap checks
drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
selinux: use vma_is_initial_stack() and vma_is_initial_heap()
perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
fs/proc/task_mmu.c | 24 ++++----------------
fs/proc/task_nommu.c | 15 +------------
include/linux/mm.h | 25 +++++++++++++++++++++
kernel/events/core.c | 33 ++++++++++------------------
security/selinux/hooks.c | 7 ++----
6 files changed, 44 insertions(+), 65 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, eparis,
linux-fsdevel, Alex Deucher, acme, christian.koenig,
Christian Göttsche
Add vma_is_initial_stack() and vma_is_initial_heap() helper and use
them to simplify code.
v2:
- add comment for heap helper and remove one more goto cpy_name,
per David Hildenbrand
- add RB
v2:
- address comments per David Hildenbrand and Christian Göttsche
- fix selinux build
Kefeng Wang (4):
mm: factor out VMA stack and heap checks
drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
selinux: use vma_is_initial_stack() and vma_is_initial_heap()
perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
fs/proc/task_mmu.c | 24 ++++----------------
fs/proc/task_nommu.c | 15 +------------
include/linux/mm.h | 25 +++++++++++++++++++++
kernel/events/core.c | 33 ++++++++++------------------
security/selinux/hooks.c | 7 ++----
6 files changed, 44 insertions(+), 65 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, acme, airlied, christian.koenig,
Christian Göttsche
Add vma_is_initial_stack() and vma_is_initial_heap() helper and use
them to simplify code.
v2:
- add comment for heap helper and remove one more goto cpy_name,
per David Hildenbrand
- add RB
v2:
- address comments per David Hildenbrand and Christian Göttsche
- fix selinux build
Kefeng Wang (4):
mm: factor out VMA stack and heap checks
drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
selinux: use vma_is_initial_stack() and vma_is_initial_heap()
perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
fs/proc/task_mmu.c | 24 ++++----------------
fs/proc/task_nommu.c | 15 +------------
include/linux/mm.h | 25 +++++++++++++++++++++
kernel/events/core.c | 33 ++++++++++------------------
security/selinux/hooks.c | 7 ++----
6 files changed, 44 insertions(+), 65 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] mm: factor out VMA stack and heap checks
2023-07-28 5:00 ` Kefeng Wang
(?)
@ 2023-07-28 5:00 ` Kefeng Wang
-1 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: amd-gfx, dri-devel, linux-kernel, linux-fsdevel, linux-mm,
linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, peterz, acme, Kefeng Wang
Factor out VMA stack and heap checks and name them
vma_is_initial_stack() and vma_is_initial_heap() for
general use.
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
fs/proc/task_mmu.c | 24 ++++--------------------
fs/proc/task_nommu.c | 15 +--------------
include/linux/mm.h | 25 +++++++++++++++++++++++++
3 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2b434b3fa2ec..3f063201b1ec 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -236,21 +236,6 @@ static int do_maps_open(struct inode *inode, struct file *file,
sizeof(struct proc_maps_private));
}
-/*
- * Indicate if the VMA is a stack for the given task; for
- * /proc/PID/maps that is the stack of the main task.
- */
-static int is_stack(struct vm_area_struct *vma)
-{
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack;
-}
-
static void show_vma_header_prefix(struct seq_file *m,
unsigned long start, unsigned long end,
vm_flags_t flags, unsigned long long pgoff,
@@ -327,13 +312,12 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
goto done;
}
- if (vma->vm_start <= mm->brk &&
- vma->vm_end >= mm->start_brk) {
+ if (vma_is_initial_heap(vma)) {
name = "[heap]";
goto done;
}
- if (is_stack(vma)) {
+ if (vma_is_initial_stack(vma)) {
name = "[stack]";
goto done;
}
@@ -1975,9 +1959,9 @@ static int show_numa_map(struct seq_file *m, void *v)
if (file) {
seq_puts(m, " file=");
seq_file_path(m, file, "\n\t= ");
- } else if (vma->vm_start <= mm->brk && vma->vm_end >= mm->start_brk) {
+ } else if (vma_is_initial_heap(vma)) {
seq_puts(m, " heap");
- } else if (is_stack(vma)) {
+ } else if (vma_is_initial_stack(vma)) {
seq_puts(m, " stack");
}
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 2c8b62265981..a8ac0dd8041e 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -121,19 +121,6 @@ unsigned long task_statm(struct mm_struct *mm,
return size;
}
-static int is_stack(struct vm_area_struct *vma)
-{
- struct mm_struct *mm = vma->vm_mm;
-
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= mm->start_stack &&
- vma->vm_end >= mm->start_stack;
-}
-
/*
* display a single VMA to a sequenced file
*/
@@ -171,7 +158,7 @@ static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
if (file) {
seq_pad(m, ' ');
seq_file_path(m, file, "");
- } else if (mm && is_stack(vma)) {
+ } else if (mm && vma_is_initial_stack(vma)) {
seq_pad(m, ' ');
seq_puts(m, "[stack]");
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index acaef8d106f0..2fbc6c631764 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -866,6 +866,31 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
return !vma->vm_ops;
}
+/*
+ * Indicate if the VMA is a heap for the given task; for
+ * /proc/PID/maps that is the heap of the main task.
+ */
+static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
+{
+ return vma->vm_start <= vma->vm_mm->brk &&
+ vma->vm_end >= vma->vm_mm->start_brk;
+}
+
+/*
+ * Indicate if the VMA is a stack for the given task; for
+ * /proc/PID/maps that is the stack of the main task.
+ */
+static inline bool vma_is_initial_stack(const struct vm_area_struct *vma)
+{
+ /*
+ * We make no effort to guess what a given thread considers to be
+ * its "stack". It's not even well-defined for programs written
+ * languages like Go.
+ */
+ return vma->vm_start <= vma->vm_mm->start_stack &&
+ vma->vm_end >= vma->vm_mm->start_stack;
+}
+
static inline bool vma_is_temporary_stack(struct vm_area_struct *vma)
{
int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] mm: factor out VMA stack and heap checks
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, eparis,
linux-fsdevel, Alex Deucher, acme, christian.koenig,
Christian Göttsche
Factor out VMA stack and heap checks and name them
vma_is_initial_stack() and vma_is_initial_heap() for
general use.
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
fs/proc/task_mmu.c | 24 ++++--------------------
fs/proc/task_nommu.c | 15 +--------------
include/linux/mm.h | 25 +++++++++++++++++++++++++
3 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2b434b3fa2ec..3f063201b1ec 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -236,21 +236,6 @@ static int do_maps_open(struct inode *inode, struct file *file,
sizeof(struct proc_maps_private));
}
-/*
- * Indicate if the VMA is a stack for the given task; for
- * /proc/PID/maps that is the stack of the main task.
- */
-static int is_stack(struct vm_area_struct *vma)
-{
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack;
-}
-
static void show_vma_header_prefix(struct seq_file *m,
unsigned long start, unsigned long end,
vm_flags_t flags, unsigned long long pgoff,
@@ -327,13 +312,12 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
goto done;
}
- if (vma->vm_start <= mm->brk &&
- vma->vm_end >= mm->start_brk) {
+ if (vma_is_initial_heap(vma)) {
name = "[heap]";
goto done;
}
- if (is_stack(vma)) {
+ if (vma_is_initial_stack(vma)) {
name = "[stack]";
goto done;
}
@@ -1975,9 +1959,9 @@ static int show_numa_map(struct seq_file *m, void *v)
if (file) {
seq_puts(m, " file=");
seq_file_path(m, file, "\n\t= ");
- } else if (vma->vm_start <= mm->brk && vma->vm_end >= mm->start_brk) {
+ } else if (vma_is_initial_heap(vma)) {
seq_puts(m, " heap");
- } else if (is_stack(vma)) {
+ } else if (vma_is_initial_stack(vma)) {
seq_puts(m, " stack");
}
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 2c8b62265981..a8ac0dd8041e 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -121,19 +121,6 @@ unsigned long task_statm(struct mm_struct *mm,
return size;
}
-static int is_stack(struct vm_area_struct *vma)
-{
- struct mm_struct *mm = vma->vm_mm;
-
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= mm->start_stack &&
- vma->vm_end >= mm->start_stack;
-}
-
/*
* display a single VMA to a sequenced file
*/
@@ -171,7 +158,7 @@ static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
if (file) {
seq_pad(m, ' ');
seq_file_path(m, file, "");
- } else if (mm && is_stack(vma)) {
+ } else if (mm && vma_is_initial_stack(vma)) {
seq_pad(m, ' ');
seq_puts(m, "[stack]");
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index acaef8d106f0..2fbc6c631764 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -866,6 +866,31 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
return !vma->vm_ops;
}
+/*
+ * Indicate if the VMA is a heap for the given task; for
+ * /proc/PID/maps that is the heap of the main task.
+ */
+static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
+{
+ return vma->vm_start <= vma->vm_mm->brk &&
+ vma->vm_end >= vma->vm_mm->start_brk;
+}
+
+/*
+ * Indicate if the VMA is a stack for the given task; for
+ * /proc/PID/maps that is the stack of the main task.
+ */
+static inline bool vma_is_initial_stack(const struct vm_area_struct *vma)
+{
+ /*
+ * We make no effort to guess what a given thread considers to be
+ * its "stack". It's not even well-defined for programs written
+ * languages like Go.
+ */
+ return vma->vm_start <= vma->vm_mm->start_stack &&
+ vma->vm_end >= vma->vm_mm->start_stack;
+}
+
static inline bool vma_is_temporary_stack(struct vm_area_struct *vma)
{
int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 1/4] mm: factor out VMA stack and heap checks
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, acme, airlied, christian.koenig,
Christian Göttsche
Factor out VMA stack and heap checks and name them
vma_is_initial_stack() and vma_is_initial_heap() for
general use.
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
fs/proc/task_mmu.c | 24 ++++--------------------
fs/proc/task_nommu.c | 15 +--------------
include/linux/mm.h | 25 +++++++++++++++++++++++++
3 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2b434b3fa2ec..3f063201b1ec 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -236,21 +236,6 @@ static int do_maps_open(struct inode *inode, struct file *file,
sizeof(struct proc_maps_private));
}
-/*
- * Indicate if the VMA is a stack for the given task; for
- * /proc/PID/maps that is the stack of the main task.
- */
-static int is_stack(struct vm_area_struct *vma)
-{
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack;
-}
-
static void show_vma_header_prefix(struct seq_file *m,
unsigned long start, unsigned long end,
vm_flags_t flags, unsigned long long pgoff,
@@ -327,13 +312,12 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
goto done;
}
- if (vma->vm_start <= mm->brk &&
- vma->vm_end >= mm->start_brk) {
+ if (vma_is_initial_heap(vma)) {
name = "[heap]";
goto done;
}
- if (is_stack(vma)) {
+ if (vma_is_initial_stack(vma)) {
name = "[stack]";
goto done;
}
@@ -1975,9 +1959,9 @@ static int show_numa_map(struct seq_file *m, void *v)
if (file) {
seq_puts(m, " file=");
seq_file_path(m, file, "\n\t= ");
- } else if (vma->vm_start <= mm->brk && vma->vm_end >= mm->start_brk) {
+ } else if (vma_is_initial_heap(vma)) {
seq_puts(m, " heap");
- } else if (is_stack(vma)) {
+ } else if (vma_is_initial_stack(vma)) {
seq_puts(m, " stack");
}
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 2c8b62265981..a8ac0dd8041e 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -121,19 +121,6 @@ unsigned long task_statm(struct mm_struct *mm,
return size;
}
-static int is_stack(struct vm_area_struct *vma)
-{
- struct mm_struct *mm = vma->vm_mm;
-
- /*
- * We make no effort to guess what a given thread considers to be
- * its "stack". It's not even well-defined for programs written
- * languages like Go.
- */
- return vma->vm_start <= mm->start_stack &&
- vma->vm_end >= mm->start_stack;
-}
-
/*
* display a single VMA to a sequenced file
*/
@@ -171,7 +158,7 @@ static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
if (file) {
seq_pad(m, ' ');
seq_file_path(m, file, "");
- } else if (mm && is_stack(vma)) {
+ } else if (mm && vma_is_initial_stack(vma)) {
seq_pad(m, ' ');
seq_puts(m, "[stack]");
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index acaef8d106f0..2fbc6c631764 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -866,6 +866,31 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
return !vma->vm_ops;
}
+/*
+ * Indicate if the VMA is a heap for the given task; for
+ * /proc/PID/maps that is the heap of the main task.
+ */
+static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
+{
+ return vma->vm_start <= vma->vm_mm->brk &&
+ vma->vm_end >= vma->vm_mm->start_brk;
+}
+
+/*
+ * Indicate if the VMA is a stack for the given task; for
+ * /proc/PID/maps that is the stack of the main task.
+ */
+static inline bool vma_is_initial_stack(const struct vm_area_struct *vma)
+{
+ /*
+ * We make no effort to guess what a given thread considers to be
+ * its "stack". It's not even well-defined for programs written
+ * languages like Go.
+ */
+ return vma->vm_start <= vma->vm_mm->start_stack &&
+ vma->vm_end >= vma->vm_mm->start_stack;
+}
+
static inline bool vma_is_temporary_stack(struct vm_area_struct *vma)
{
int maybe_stack = vma->vm_flags & (VM_GROWSDOWN | VM_GROWSUP);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
2023-07-28 5:00 ` Kefeng Wang
(?)
@ 2023-07-28 5:00 ` Kefeng Wang
-1 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: amd-gfx, dri-devel, linux-kernel, linux-fsdevel, linux-mm,
linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, peterz, acme, Kefeng Wang
Use the helpers to simplify code.
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 01c7de2d6e19..308384dbc502 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2624,10 +2624,7 @@ svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
return -EFAULT;
}
- *is_heap_stack = (vma->vm_start <= vma->vm_mm->brk &&
- vma->vm_end >= vma->vm_mm->start_brk) ||
- (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack);
+ *is_heap_stack = vma_is_initial_heap(vma) || vma_is_initial_stack(vma);
start_limit = max(vma->vm_start >> PAGE_SHIFT,
(unsigned long)ALIGN_DOWN(addr, 2UL << 8));
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, eparis,
linux-fsdevel, Alex Deucher, acme, christian.koenig,
Christian Göttsche
Use the helpers to simplify code.
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 01c7de2d6e19..308384dbc502 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2624,10 +2624,7 @@ svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
return -EFAULT;
}
- *is_heap_stack = (vma->vm_start <= vma->vm_mm->brk &&
- vma->vm_end >= vma->vm_mm->start_brk) ||
- (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack);
+ *is_heap_stack = vma_is_initial_heap(vma) || vma_is_initial_stack(vma);
start_limit = max(vma->vm_start >> PAGE_SHIFT,
(unsigned long)ALIGN_DOWN(addr, 2UL << 8));
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/4] drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, acme, airlied, christian.koenig,
Christian Göttsche
Use the helpers to simplify code.
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 01c7de2d6e19..308384dbc502 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2624,10 +2624,7 @@ svm_range_get_range_boundaries(struct kfd_process *p, int64_t addr,
return -EFAULT;
}
- *is_heap_stack = (vma->vm_start <= vma->vm_mm->brk &&
- vma->vm_end >= vma->vm_mm->start_brk) ||
- (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack);
+ *is_heap_stack = vma_is_initial_heap(vma) || vma_is_initial_stack(vma);
start_limit = max(vma->vm_start >> PAGE_SHIFT,
(unsigned long)ALIGN_DOWN(addr, 2UL << 8));
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-07-28 5:00 ` Kefeng Wang
(?)
@ 2023-07-28 5:00 ` Kefeng Wang
-1 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: amd-gfx, dri-devel, linux-kernel, linux-fsdevel, linux-mm,
linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, peterz, acme, Kefeng Wang
Use the helpers to simplify code.
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Eric Paris <eparis@parisplace.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
security/selinux/hooks.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c87b79a29fad..ac582c046c51 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3800,13 +3800,10 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
int rc = 0;
- if (vma->vm_start >= vma->vm_mm->start_brk &&
- vma->vm_end <= vma->vm_mm->brk) {
+ if (vma_is_initial_heap(vma)) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file &&
- ((vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) ||
+ } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, eparis,
linux-fsdevel, Alex Deucher, acme, christian.koenig,
Christian Göttsche
Use the helpers to simplify code.
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Eric Paris <eparis@parisplace.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
security/selinux/hooks.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c87b79a29fad..ac582c046c51 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3800,13 +3800,10 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
int rc = 0;
- if (vma->vm_start >= vma->vm_mm->start_brk &&
- vma->vm_end <= vma->vm_mm->brk) {
+ if (vma_is_initial_heap(vma)) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file &&
- ((vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) ||
+ } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, acme, airlied, christian.koenig,
Christian Göttsche
Use the helpers to simplify code.
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Eric Paris <eparis@parisplace.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
security/selinux/hooks.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c87b79a29fad..ac582c046c51 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3800,13 +3800,10 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
int rc = 0;
- if (vma->vm_start >= vma->vm_mm->start_brk &&
- vma->vm_end <= vma->vm_mm->brk) {
+ if (vma_is_initial_heap(vma)) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file &&
- ((vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) ||
+ } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 4/4] perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
2023-07-28 5:00 ` Kefeng Wang
(?)
@ 2023-07-28 5:00 ` Kefeng Wang
-1 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: amd-gfx, dri-devel, linux-kernel, linux-fsdevel, linux-mm,
linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, peterz, acme, Kefeng Wang
Use the helpers to simplify code, also kill unneeded goto cpy_name.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
kernel/events/core.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f84e2640ea2f..b36d0823ff33 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8631,7 +8631,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
unsigned int size;
char tmp[16];
char *buf = NULL;
- char *name;
+ char *name = NULL;
if (vma->vm_flags & VM_READ)
prot |= PROT_READ;
@@ -8678,29 +8678,18 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
goto got_name;
} else {
- if (vma->vm_ops && vma->vm_ops->name) {
+ if (vma->vm_ops && vma->vm_ops->name)
name = (char *) vma->vm_ops->name(vma);
- if (name)
- goto cpy_name;
+ if (!name)
+ name = (char *)arch_vma_name(vma);
+ if (!name) {
+ if (vma_is_initial_heap(vma))
+ name = "[heap]";
+ else if (vma_is_initial_stack(vma))
+ name = "[stack]";
+ else
+ name = "//anon";
}
-
- name = (char *)arch_vma_name(vma);
- if (name)
- goto cpy_name;
-
- if (vma->vm_start <= vma->vm_mm->start_brk &&
- vma->vm_end >= vma->vm_mm->brk) {
- name = "[heap]";
- goto cpy_name;
- }
- if (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) {
- name = "[stack]";
- goto cpy_name;
- }
-
- name = "//anon";
- goto cpy_name;
}
cpy_name:
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 4/4] perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, eparis,
linux-fsdevel, Alex Deucher, acme, christian.koenig,
Christian Göttsche
Use the helpers to simplify code, also kill unneeded goto cpy_name.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
kernel/events/core.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f84e2640ea2f..b36d0823ff33 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8631,7 +8631,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
unsigned int size;
char tmp[16];
char *buf = NULL;
- char *name;
+ char *name = NULL;
if (vma->vm_flags & VM_READ)
prot |= PROT_READ;
@@ -8678,29 +8678,18 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
goto got_name;
} else {
- if (vma->vm_ops && vma->vm_ops->name) {
+ if (vma->vm_ops && vma->vm_ops->name)
name = (char *) vma->vm_ops->name(vma);
- if (name)
- goto cpy_name;
+ if (!name)
+ name = (char *)arch_vma_name(vma);
+ if (!name) {
+ if (vma_is_initial_heap(vma))
+ name = "[heap]";
+ else if (vma_is_initial_stack(vma))
+ name = "[stack]";
+ else
+ name = "//anon";
}
-
- name = (char *)arch_vma_name(vma);
- if (name)
- goto cpy_name;
-
- if (vma->vm_start <= vma->vm_mm->start_brk &&
- vma->vm_end >= vma->vm_mm->brk) {
- name = "[heap]";
- goto cpy_name;
- }
- if (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) {
- name = "[stack]";
- goto cpy_name;
- }
-
- name = "//anon";
- goto cpy_name;
}
cpy_name:
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 4/4] perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
@ 2023-07-28 5:00 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-07-28 5:00 UTC (permalink / raw)
To: Andrew Morton
Cc: stephen.smalley.work, Kefeng Wang, paul, David Hildenbrand,
selinux, Felix Kuehling, Xinhui.Pan, linux-kernel, dri-devel,
linux-perf-users, linux-mm, peterz, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, acme, airlied, christian.koenig,
Christian Göttsche
Use the helpers to simplify code, also kill unneeded goto cpy_name.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
kernel/events/core.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f84e2640ea2f..b36d0823ff33 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8631,7 +8631,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
unsigned int size;
char tmp[16];
char *buf = NULL;
- char *name;
+ char *name = NULL;
if (vma->vm_flags & VM_READ)
prot |= PROT_READ;
@@ -8678,29 +8678,18 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
goto got_name;
} else {
- if (vma->vm_ops && vma->vm_ops->name) {
+ if (vma->vm_ops && vma->vm_ops->name)
name = (char *) vma->vm_ops->name(vma);
- if (name)
- goto cpy_name;
+ if (!name)
+ name = (char *)arch_vma_name(vma);
+ if (!name) {
+ if (vma_is_initial_heap(vma))
+ name = "[heap]";
+ else if (vma_is_initial_stack(vma))
+ name = "[stack]";
+ else
+ name = "//anon";
}
-
- name = (char *)arch_vma_name(vma);
- if (name)
- goto cpy_name;
-
- if (vma->vm_start <= vma->vm_mm->start_brk &&
- vma->vm_end >= vma->vm_mm->brk) {
- name = "[heap]";
- goto cpy_name;
- }
- if (vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) {
- name = "[stack]";
- goto cpy_name;
- }
-
- name = "//anon";
- goto cpy_name;
}
cpy_name:
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
2023-07-28 5:00 ` Kefeng Wang
(?)
@ 2023-07-31 13:47 ` Peter Zijlstra
-1 siblings, 0 replies; 25+ messages in thread
From: Peter Zijlstra @ 2023-07-31 13:47 UTC (permalink / raw)
To: Kefeng Wang
Cc: Andrew Morton, amd-gfx, dri-devel, linux-kernel, linux-fsdevel,
linux-mm, linux-perf-users, selinux, Christian Göttsche,
David Hildenbrand, Felix Kuehling, Alex Deucher,
christian.koenig, Xinhui.Pan, airlied, daniel, paul,
stephen.smalley.work, eparis, acme
On Fri, Jul 28, 2023 at 01:00:39PM +0800, Kefeng Wang wrote:
> Kefeng Wang (4):
> mm: factor out VMA stack and heap checks
> drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
> selinux: use vma_is_initial_stack() and vma_is_initial_heap()
> perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
>
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
> fs/proc/task_mmu.c | 24 ++++----------------
> fs/proc/task_nommu.c | 15 +------------
> include/linux/mm.h | 25 +++++++++++++++++++++
> kernel/events/core.c | 33 ++++++++++------------------
> security/selinux/hooks.c | 7 ++----
> 6 files changed, 44 insertions(+), 65 deletions(-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
@ 2023-07-31 13:47 ` Peter Zijlstra
0 siblings, 0 replies; 25+ messages in thread
From: Peter Zijlstra @ 2023-07-31 13:47 UTC (permalink / raw)
To: Kefeng Wang
Cc: stephen.smalley.work, paul, David Hildenbrand, selinux,
Felix Kuehling, Xinhui.Pan, acme, linux-kernel, dri-devel,
linux-perf-users, linux-mm, amd-gfx, eparis, linux-fsdevel,
Alex Deucher, Andrew Morton, christian.koenig,
Christian Göttsche
On Fri, Jul 28, 2023 at 01:00:39PM +0800, Kefeng Wang wrote:
> Kefeng Wang (4):
> mm: factor out VMA stack and heap checks
> drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
> selinux: use vma_is_initial_stack() and vma_is_initial_heap()
> perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
>
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
> fs/proc/task_mmu.c | 24 ++++----------------
> fs/proc/task_nommu.c | 15 +------------
> include/linux/mm.h | 25 +++++++++++++++++++++
> kernel/events/core.c | 33 ++++++++++------------------
> security/selinux/hooks.c | 7 ++----
> 6 files changed, 44 insertions(+), 65 deletions(-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack()
@ 2023-07-31 13:47 ` Peter Zijlstra
0 siblings, 0 replies; 25+ messages in thread
From: Peter Zijlstra @ 2023-07-31 13:47 UTC (permalink / raw)
To: Kefeng Wang
Cc: stephen.smalley.work, paul, David Hildenbrand, selinux,
Felix Kuehling, Xinhui.Pan, acme, linux-kernel, dri-devel,
linux-perf-users, linux-mm, amd-gfx, daniel, eparis,
linux-fsdevel, Alex Deucher, Andrew Morton, airlied,
christian.koenig, Christian Göttsche
On Fri, Jul 28, 2023 at 01:00:39PM +0800, Kefeng Wang wrote:
> Kefeng Wang (4):
> mm: factor out VMA stack and heap checks
> drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap()
> selinux: use vma_is_initial_stack() and vma_is_initial_heap()
> perf/core: use vma_is_initial_stack() and vma_is_initial_heap()
>
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +----
> fs/proc/task_mmu.c | 24 ++++----------------
> fs/proc/task_nommu.c | 15 +------------
> include/linux/mm.h | 25 +++++++++++++++++++++
> kernel/events/core.c | 33 ++++++++++------------------
> security/selinux/hooks.c | 7 ++----
> 6 files changed, 44 insertions(+), 65 deletions(-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Fwd: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-07-28 5:00 ` Kefeng Wang
(?)
(?)
@ 2023-07-31 14:26 ` Stephen Smalley
2023-07-31 16:19 ` Paul Moore
-1 siblings, 1 reply; 25+ messages in thread
From: Stephen Smalley @ 2023-07-31 14:26 UTC (permalink / raw)
To: Paul Moore, Ondrej Mosnacek, SElinux list
I believe this patch yields a semantic change in the SELinux execheap
permission check. That said, I think the change is for the better. For
ease of comparison, is_initial_heap() is defined in patch 1 of the
series as:
+/*
+ * Indicate if the VMA is a heap for the given task; for
+ * /proc/PID/maps that is the heap of the main task.
+ */
+static inline bool vma_is_initial_heap(const struct vm_area_struct *vma)
+{
+ return vma->vm_start <= vma->vm_mm->brk &&
+ vma->vm_end >= vma->vm_mm->start_brk;
+}
+
This is a check for whether the mapping has a non-empty intersection
with the heap range.
Whereas the existing test in the SELinux code only appears to check
whether the mapping is _within_ the heap range.
---------- Forwarded message ---------
From: Kefeng Wang <wangkefeng.wang@huawei.com>
Date: Fri, Jul 28, 2023 at 12:48 AM
Subject: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and
vma_is_initial_heap()
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <amd-gfx@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
<linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>,
<linux-perf-users@vger.kernel.org>, <selinux@vger.kernel.org>,
Christian Göttsche <cgzones@googlemail.com>, David Hildenbrand
<david@redhat.com>, Felix Kuehling <Felix.Kuehling@amd.com>, Alex
Deucher <alexander.deucher@amd.com>, <christian.koenig@amd.com>,
<Xinhui.Pan@amd.com>, <airlied@gmail.com>, <daniel@ffwll.ch>,
<paul@paul-moore.com>, <stephen.smalley.work@gmail.com>,
<eparis@parisplace.org>, <peterz@infradead.org>, <acme@kernel.org>,
Kefeng Wang <wangkefeng.wang@huawei.com>
Use the helpers to simplify code.
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Eric Paris <eparis@parisplace.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
security/selinux/hooks.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c87b79a29fad..ac582c046c51 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3800,13 +3800,10 @@ static int selinux_file_mprotect(struct
vm_area_struct *vma,
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
int rc = 0;
- if (vma->vm_start >= vma->vm_mm->start_brk &&
- vma->vm_end <= vma->vm_mm->brk) {
+ if (vma_is_initial_heap(vma)) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file &&
- ((vma->vm_start <= vma->vm_mm->start_stack &&
- vma->vm_end >= vma->vm_mm->start_stack) ||
+ } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
--
2.41.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-07-31 14:26 ` Fwd: " Stephen Smalley
@ 2023-07-31 16:19 ` Paul Moore
[not found] ` <CAEjxPJ6iFRZUetSvMgZvq_327U_JZ_w9s=gFccKgJhfCt8bqNg@mail.gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Paul Moore @ 2023-07-31 16:19 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Ondrej Mosnacek, SElinux list
On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> I believe this patch yields a semantic change in the SELinux execheap
> permission check. That said, I think the change is for the better.
Agreed. I'm also in favor of using a helper which is maintained by
the VM folks over open coded logic in the SELinux code.
--
paul-moore.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
[not found] ` <CAHC9VhRB1uVVWFUgnMZ1iwCD_A0mEX2Xhn79qTxuNKTzisWULg@mail.gmail.com>
@ 2023-12-06 14:22 ` Ondrej Mosnacek
2023-12-06 20:47 ` Paul Moore
0 siblings, 1 reply; 25+ messages in thread
From: Ondrej Mosnacek @ 2023-12-06 14:22 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, SElinux list, open list:MEMORY MANAGEMENT,
Kefeng Wang, David Hildenbrand, peterz, Andrew Morton
(Restoring part of the Cc list to include more relevant lists &
people... If you are lost, the original email is here:
https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang@huawei.com/)
On Tue, Aug 1, 2023 at 1:08 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Mon, Jul 31, 2023 at 12:19 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > > I believe this patch yields a semantic change in the SELinux execheap
> > > > permission check. That said, I think the change is for the better.
> > >
> > > Agreed. I'm also in favor of using a helper which is maintained by
> > > the VM folks over open coded logic in the SELinux code.
> >
> > Yes, only caveat is in theory it could trigger new execheap denials
> > under existing policies.
> > Trying to construct an example based on the
> > selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empty
> > so far on something that both works and yields different results
> > before and after this patch.
>
> My gut feeling is that this will not be an issue, but I could very
> well be wrong. If it becomes a significant issue we can revert the
> SELinux portion of the patch.
>
> Of course, if you have any luck demonstrating this with reasonable
> code, that would be good input too.
So, it turns out this does affect actual code. Thus far, we know about
gcl [1] and wine [2]. The gcl case is easy to reproduce (just install
gcl on Fedora and run gcl without arguments), so I was able to dig a
bit deeper.
gcl has the following relevant memory mappings as captured by gdb:
Start Addr End Addr Size Offset Perms objfile
0x413000 0xf75000 0xb62000 0x3fa000 rw-p
/usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl
0xf75000 0xf79000 0x4000 0x0 rwxp [heap]
It tries to call mprotect(0x883000, 7282688,
PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region
0x883000 - 0xf75000 executable. Before this patch it was allowed,
whereas now it triggers an execheap SELinux denial. But this seems
wrong - the affected region is merely adjacent to the [heap] region,
it does not actually overlap with it. So even if we accept that the
correct semantics is to catch any region that overlaps with the heap
(before only subregions of the heap were subject to the execheap
check), this corner case doesn't seem to be handled correctly by the
new check (and the same bug seems to have been in fs/proc/task_mmu.c
before commit 11250fd12eb8 ("mm: factor out VMA stack and heap
checks")).
I didn't analyze the wine case ([2]), but it may be the same situation.
Unless I'm mistaken, those <= & >= in should in fact be just < & >.
And the expression in vma_is_initial_stack() is also suspicious (but
I'm not going to make any assumption on what is the intended semantics
there...)
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2252391
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-12-06 14:22 ` Ondrej Mosnacek
@ 2023-12-06 20:47 ` Paul Moore
2023-12-07 4:50 ` Kefeng Wang
0 siblings, 1 reply; 25+ messages in thread
From: Paul Moore @ 2023-12-06 20:47 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Stephen Smalley, SElinux list, open list:MEMORY MANAGEMENT,
Kefeng Wang, David Hildenbrand, peterz, Andrew Morton
On Wed, Dec 6, 2023 at 9:22 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> (Restoring part of the Cc list to include more relevant lists &
> people... If you are lost, the original email is here:
> https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang@huawei.com/)
>
> On Tue, Aug 1, 2023 at 1:08 AM Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > On Mon, Jul 31, 2023 at 12:19 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
> > > > <stephen.smalley.work@gmail.com> wrote:
> > > > > I believe this patch yields a semantic change in the SELinux execheap
> > > > > permission check. That said, I think the change is for the better.
> > > >
> > > > Agreed. I'm also in favor of using a helper which is maintained by
> > > > the VM folks over open coded logic in the SELinux code.
> > >
> > > Yes, only caveat is in theory it could trigger new execheap denials
> > > under existing policies.
> > > Trying to construct an example based on the
> > > selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empty
> > > so far on something that both works and yields different results
> > > before and after this patch.
> >
> > My gut feeling is that this will not be an issue, but I could very
> > well be wrong. If it becomes a significant issue we can revert the
> > SELinux portion of the patch.
> >
> > Of course, if you have any luck demonstrating this with reasonable
> > code, that would be good input too.
>
> So, it turns out this does affect actual code. Thus far, we know about
> gcl [1] and wine [2]. The gcl case is easy to reproduce (just install
> gcl on Fedora and run gcl without arguments), so I was able to dig a
> bit deeper.
>
> gcl has the following relevant memory mappings as captured by gdb:
> Start Addr End Addr Size Offset Perms objfile
> 0x413000 0xf75000 0xb62000 0x3fa000 rw-p
> /usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl
> 0xf75000 0xf79000 0x4000 0x0 rwxp [heap]
>
> It tries to call mprotect(0x883000, 7282688,
> PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region
> 0x883000 - 0xf75000 executable. Before this patch it was allowed,
> whereas now it triggers an execheap SELinux denial. But this seems
> wrong - the affected region is merely adjacent to the [heap] region,
> it does not actually overlap with it. So even if we accept that the
> correct semantics is to catch any region that overlaps with the heap
> (before only subregions of the heap were subject to the execheap
> check), this corner case doesn't seem to be handled correctly by the
> new check (and the same bug seems to have been in fs/proc/task_mmu.c
> before commit 11250fd12eb8 ("mm: factor out VMA stack and heap
> checks")).
>
> I didn't analyze the wine case ([2]), but it may be the same situation.
>
> Unless I'm mistaken, those <= & >= in should in fact be just < & >.
> And the expression in vma_is_initial_stack() is also suspicious (but
> I'm not going to make any assumption on what is the intended semantics
> there...)
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2252391
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299
Thanks Ondrej.
I'm hoping the mm folks will comment on this as it looks like this is
an issue with the helper functions, but just in case I'm going to prep
a revert for just the SELinux changes. If we don't hear anything in
the next couple of days I'll send the revert up to Linus with the idea
that we can eventually shift back to the helpers when this is sorted.
--
paul-moore.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-12-06 20:47 ` Paul Moore
@ 2023-12-07 4:50 ` Kefeng Wang
2023-12-07 8:37 ` Ondrej Mosnacek
0 siblings, 1 reply; 25+ messages in thread
From: Kefeng Wang @ 2023-12-07 4:50 UTC (permalink / raw)
To: Paul Moore, Ondrej Mosnacek
Cc: Stephen Smalley, SElinux list, open list:MEMORY MANAGEMENT,
David Hildenbrand, peterz, Andrew Morton
On 2023/12/7 4:47, Paul Moore wrote:
> On Wed, Dec 6, 2023 at 9:22 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>
>> (Restoring part of the Cc list to include more relevant lists &
>> people... If you are lost, the original email is here:
>> https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang@huawei.com/)
>>
>> On Tue, Aug 1, 2023 at 1:08 AM Paul Moore <paul@paul-moore.com> wrote:
>>> On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley
>>> <stephen.smalley.work@gmail.com> wrote:
>>>> On Mon, Jul 31, 2023 at 12:19 PM Paul Moore <paul@paul-moore.com> wrote:
>>>>>
>>>>> On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
>>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>>> I believe this patch yields a semantic change in the SELinux execheap
>>>>>> permission check. That said, I think the change is for the better.
>>>>>
>>>>> Agreed. I'm also in favor of using a helper which is maintained by
>>>>> the VM folks over open coded logic in the SELinux code.
>>>>
>>>> Yes, only caveat is in theory it could trigger new execheap denials
>>>> under existing policies.
>>>> Trying to construct an example based on the
>>>> selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empty
>>>> so far on something that both works and yields different results
>>>> before and after this patch.
>>>
>>> My gut feeling is that this will not be an issue, but I could very
>>> well be wrong. If it becomes a significant issue we can revert the
>>> SELinux portion of the patch.
>>>
>>> Of course, if you have any luck demonstrating this with reasonable
>>> code, that would be good input too.
>>
>> So, it turns out this does affect actual code. Thus far, we know about
>> gcl [1] and wine [2]. The gcl case is easy to reproduce (just install
>> gcl on Fedora and run gcl without arguments), so I was able to dig a
>> bit deeper.
>>
>> gcl has the following relevant memory mappings as captured by gdb:
>> Start Addr End Addr Size Offset Perms objfile
>> 0x413000 0xf75000 0xb62000 0x3fa000 rw-p
>> /usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl
>> 0xf75000 0xf79000 0x4000 0x0 rwxp [heap]
>>
>> It tries to call mprotect(0x883000, 7282688,
>> PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region
>> 0x883000 - 0xf75000 executable. Before this patch it was allowed,
>> whereas now it triggers an execheap SELinux denial. But this seems
>> wrong - the affected region is merely adjacent to the [heap] region,
>> it does not actually overlap with it. So even if we accept that the
>> correct semantics is to catch any region that overlaps with the heap
>> (before only subregions of the heap were subject to the execheap
>> check), this corner case doesn't seem to be handled correctly by the
>> new check (and the same bug seems to have been in fs/proc/task_mmu.c
>> before commit 11250fd12eb8 ("mm: factor out VMA stack and heap
>> checks")).
Yes, the heap check exists for a long time,
[start_brk brk]
case1: [vm_start,vm_end]
case2: [vm_start,vm_end]
case3: [vm_start,vm_end]
case4: [vm_start, vm_end]
case5: [vm_start,vm_end]
case6: [vm_start,vm_end]
old check:
vma->vm_start >= vma->vm_mm->start_brk && vma->vm_end <= vma->vm_mm->brk
Only include case1, vma range must be within heap
new check:
vma->vm_start <= vma->vm_mm->brk && vma->vm_end >= vma->vm_mm->start_brk
Include case1~case6, but case5(vm_end=start_brk) and case6(vm_start=brk)
are the corner cases, gcl issue matchs the case5.
>>
>> I didn't analyze the wine case ([2]), but it may be the same situation.
>>
>> Unless I'm mistaken, those <= & >= in should in fact be just < & >.
I support this change.
vma->vm_start < vma->vm_mm->brk && vma->vm_end > vma->vm_mm->start_brk
>> And the expression in vma_is_initial_stack() is also suspicious (but
>> I'm not going to make any assumption on what is the intended semantics
>> there...)
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2252391
>> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299
Could you quickly verify after the above change?
>
> Thanks Ondrej.
>
> I'm hoping the mm folks will comment on this as it looks like this is
> an issue with the helper functions, but just in case I'm going to prep
> a revert for just the SELinux changes. If we don't hear anything in
> the next couple of days I'll send the revert up to Linus with the idea
> that we can eventually shift back to the helpers when this is sorted.
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-12-07 4:50 ` Kefeng Wang
@ 2023-12-07 8:37 ` Ondrej Mosnacek
2023-12-07 9:23 ` Kefeng Wang
0 siblings, 1 reply; 25+ messages in thread
From: Ondrej Mosnacek @ 2023-12-07 8:37 UTC (permalink / raw)
To: Kefeng Wang
Cc: Paul Moore, Stephen Smalley, SElinux list,
open list:MEMORY MANAGEMENT, David Hildenbrand, peterz,
Andrew Morton
On Thu, Dec 7, 2023 at 5:50 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
>
>
> On 2023/12/7 4:47, Paul Moore wrote:
> > On Wed, Dec 6, 2023 at 9:22 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >>
> >> (Restoring part of the Cc list to include more relevant lists &
> >> people... If you are lost, the original email is here:
> >> https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang@huawei.com/)
> >>
> >> On Tue, Aug 1, 2023 at 1:08 AM Paul Moore <paul@paul-moore.com> wrote:
> >>> On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley
> >>> <stephen.smalley.work@gmail.com> wrote:
> >>>> On Mon, Jul 31, 2023 at 12:19 PM Paul Moore <paul@paul-moore.com> wrote:
> >>>>>
> >>>>> On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
> >>>>> <stephen.smalley.work@gmail.com> wrote:
> >>>>>> I believe this patch yields a semantic change in the SELinux execheap
> >>>>>> permission check. That said, I think the change is for the better.
> >>>>>
> >>>>> Agreed. I'm also in favor of using a helper which is maintained by
> >>>>> the VM folks over open coded logic in the SELinux code.
> >>>>
> >>>> Yes, only caveat is in theory it could trigger new execheap denials
> >>>> under existing policies.
> >>>> Trying to construct an example based on the
> >>>> selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empty
> >>>> so far on something that both works and yields different results
> >>>> before and after this patch.
> >>>
> >>> My gut feeling is that this will not be an issue, but I could very
> >>> well be wrong. If it becomes a significant issue we can revert the
> >>> SELinux portion of the patch.
> >>>
> >>> Of course, if you have any luck demonstrating this with reasonable
> >>> code, that would be good input too.
> >>
> >> So, it turns out this does affect actual code. Thus far, we know about
> >> gcl [1] and wine [2]. The gcl case is easy to reproduce (just install
> >> gcl on Fedora and run gcl without arguments), so I was able to dig a
> >> bit deeper.
> >>
> >> gcl has the following relevant memory mappings as captured by gdb:
> >> Start Addr End Addr Size Offset Perms objfile
> >> 0x413000 0xf75000 0xb62000 0x3fa000 rw-p
> >> /usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl
> >> 0xf75000 0xf79000 0x4000 0x0 rwxp [heap]
> >>
> >> It tries to call mprotect(0x883000, 7282688,
> >> PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region
> >> 0x883000 - 0xf75000 executable. Before this patch it was allowed,
> >> whereas now it triggers an execheap SELinux denial. But this seems
> >> wrong - the affected region is merely adjacent to the [heap] region,
> >> it does not actually overlap with it. So even if we accept that the
> >> correct semantics is to catch any region that overlaps with the heap
> >> (before only subregions of the heap were subject to the execheap
> >> check), this corner case doesn't seem to be handled correctly by the
> >> new check (and the same bug seems to have been in fs/proc/task_mmu.c
> >> before commit 11250fd12eb8 ("mm: factor out VMA stack and heap
> >> checks")).
>
> Yes, the heap check exists for a long time,
>
> [start_brk brk]
> case1: [vm_start,vm_end]
> case2: [vm_start,vm_end]
> case3: [vm_start,vm_end]
> case4: [vm_start, vm_end]
>
> case5: [vm_start,vm_end]
> case6: [vm_start,vm_end]
>
> old check:
> vma->vm_start >= vma->vm_mm->start_brk && vma->vm_end <= vma->vm_mm->brk
>
> Only include case1, vma range must be within heap
>
> new check:
> vma->vm_start <= vma->vm_mm->brk && vma->vm_end >= vma->vm_mm->start_brk
>
> Include case1~case6, but case5(vm_end=start_brk) and case6(vm_start=brk)
> are the corner cases, gcl issue matchs the case5.
>
> >>
> >> I didn't analyze the wine case ([2]), but it may be the same situation.
> >>
> >> Unless I'm mistaken, those <= & >= in should in fact be just < & >.
>
> I support this change.
> vma->vm_start < vma->vm_mm->brk && vma->vm_end > vma->vm_mm->start_brk
>
> >> And the expression in vma_is_initial_stack() is also suspicious (but
> >> I'm not going to make any assumption on what is the intended semantics
> >> there...)
> >>
> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2252391
> >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299
>
> Could you quickly verify after the above change?
Yes, changing the operators as suggested fixes the gcl case.
BTW, the vma_is_*() helpers introduced by your patch also have wrong
indentation - the "return" lines are indented by 7 spaces instead of a
tab. If you are going to submit a fixing patch you might want to fix
that, too.
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap()
2023-12-07 8:37 ` Ondrej Mosnacek
@ 2023-12-07 9:23 ` Kefeng Wang
0 siblings, 0 replies; 25+ messages in thread
From: Kefeng Wang @ 2023-12-07 9:23 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Paul Moore, Stephen Smalley, SElinux list,
open list:MEMORY MANAGEMENT, David Hildenbrand, peterz,
Andrew Morton
On 2023/12/7 16:37, Ondrej Mosnacek wrote:
> On Thu, Dec 7, 2023 at 5:50 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
>>
>>
>> On 2023/12/7 4:47, Paul Moore wrote:
>>> On Wed, Dec 6, 2023 at 9:22 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>>>
>>>> (Restoring part of the Cc list to include more relevant lists &
>>>> people... If you are lost, the original email is here:
>>>> https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang@huawei.com/)
>>>>
>>>> On Tue, Aug 1, 2023 at 1:08 AM Paul Moore <paul@paul-moore.com> wrote:
>>>>> On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley
>>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>>> On Mon, Jul 31, 2023 at 12:19 PM Paul Moore <paul@paul-moore.com> wrote:
>>>>>>>
>>>>>>> On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley
>>>>>>> <stephen.smalley.work@gmail.com> wrote:
>>>>>>>> I believe this patch yields a semantic change in the SELinux execheap
>>>>>>>> permission check. That said, I think the change is for the better.
>>>>>>>
>>>>>>> Agreed. I'm also in favor of using a helper which is maintained by
>>>>>>> the VM folks over open coded logic in the SELinux code.
>>>>>>
>>>>>> Yes, only caveat is in theory it could trigger new execheap denials
>>>>>> under existing policies.
>>>>>> Trying to construct an example based on the
>>>>>> selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empty
>>>>>> so far on something that both works and yields different results
>>>>>> before and after this patch.
>>>>>
>>>>> My gut feeling is that this will not be an issue, but I could very
>>>>> well be wrong. If it becomes a significant issue we can revert the
>>>>> SELinux portion of the patch.
>>>>>
>>>>> Of course, if you have any luck demonstrating this with reasonable
>>>>> code, that would be good input too.
>>>>
>>>> So, it turns out this does affect actual code. Thus far, we know about
>>>> gcl [1] and wine [2]. The gcl case is easy to reproduce (just install
>>>> gcl on Fedora and run gcl without arguments), so I was able to dig a
>>>> bit deeper.
>>>>
>>>> gcl has the following relevant memory mappings as captured by gdb:
>>>> Start Addr End Addr Size Offset Perms objfile
>>>> 0x413000 0xf75000 0xb62000 0x3fa000 rw-p
>>>> /usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl
>>>> 0xf75000 0xf79000 0x4000 0x0 rwxp [heap]
>>>>
>>>> It tries to call mprotect(0x883000, 7282688,
>>>> PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region
>>>> 0x883000 - 0xf75000 executable. Before this patch it was allowed,
>>>> whereas now it triggers an execheap SELinux denial. But this seems
>>>> wrong - the affected region is merely adjacent to the [heap] region,
>>>> it does not actually overlap with it. So even if we accept that the
>>>> correct semantics is to catch any region that overlaps with the heap
>>>> (before only subregions of the heap were subject to the execheap
>>>> check), this corner case doesn't seem to be handled correctly by the
>>>> new check (and the same bug seems to have been in fs/proc/task_mmu.c
>>>> before commit 11250fd12eb8 ("mm: factor out VMA stack and heap
>>>> checks")).
>>
>> Yes, the heap check exists for a long time,
>>
>> [start_brk brk]
>> case1: [vm_start,vm_end]
>> case2: [vm_start,vm_end]
>> case3: [vm_start,vm_end]
>> case4: [vm_start, vm_end]
>>
>> case5: [vm_start,vm_end]
>> case6: [vm_start,vm_end]
>>
>> old check:
>> vma->vm_start >= vma->vm_mm->start_brk && vma->vm_end <= vma->vm_mm->brk
>>
>> Only include case1, vma range must be within heap
>>
>> new check:
>> vma->vm_start <= vma->vm_mm->brk && vma->vm_end >= vma->vm_mm->start_brk
>>
>> Include case1~case6, but case5(vm_end=start_brk) and case6(vm_start=brk)
>> are the corner cases, gcl issue matchs the case5.
>>
>>>>
>>>> I didn't analyze the wine case ([2]), but it may be the same situation.
>>>>
>>>> Unless I'm mistaken, those <= & >= in should in fact be just < & >.
>>
>> I support this change.
>> vma->vm_start < vma->vm_mm->brk && vma->vm_end > vma->vm_mm->start_brk
>>
>>>> And the expression in vma_is_initial_stack() is also suspicious (but
>>>> I'm not going to make any assumption on what is the intended semantics
>>>> there...)
>>>>
>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2252391
>>>> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299
>>
>> Could you quickly verify after the above change?
>
> Yes, changing the operators as suggested fixes the gcl case.
Thanks for your confirm,win[2] fixed too, right?
>
> BTW, the vma_is_*() helpers introduced by your patch also have wrong
> indentation - the "return" lines are indented by 7 spaces instead of a
> tab. If you are going to submit a fixing patch you might want to fix
> that, too.
Sure. will fix.
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2023-12-07 9:23 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-28 5:00 [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack() Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` [PATCH v3 1/4] mm: factor out VMA stack and heap checks Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` [PATCH v3 2/4] drm/amdkfd: use vma_is_initial_stack() and vma_is_initial_heap() Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` [PATCH v3 3/4] selinux: " Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-31 14:26 ` Fwd: " Stephen Smalley
2023-07-31 16:19 ` Paul Moore
[not found] ` <CAEjxPJ6iFRZUetSvMgZvq_327U_JZ_w9s=gFccKgJhfCt8bqNg@mail.gmail.com>
[not found] ` <CAHC9VhRB1uVVWFUgnMZ1iwCD_A0mEX2Xhn79qTxuNKTzisWULg@mail.gmail.com>
2023-12-06 14:22 ` Ondrej Mosnacek
2023-12-06 20:47 ` Paul Moore
2023-12-07 4:50 ` Kefeng Wang
2023-12-07 8:37 ` Ondrej Mosnacek
2023-12-07 9:23 ` Kefeng Wang
2023-07-28 5:00 ` [PATCH v3 4/4] perf/core: " Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-28 5:00 ` Kefeng Wang
2023-07-31 13:47 ` [PATCH v3 0/4] mm: convert to vma_is_initial_heap/stack() Peter Zijlstra
2023-07-31 13:47 ` Peter Zijlstra
2023-07-31 13:47 ` Peter Zijlstra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.