uprobes passes an unaligned page mapping address to
folio_add_new_anon_rmap(), which ends up triggering a VM_BUG_ON() we
recently extended in commit 372cbd4d5a066 ("mm: non-pmd-mappable, large
folios for folio_add_new_anon_rmap()").
Arguably, this is uprobes code doing something wrong; however,
for the time being it would have likely worked in rmap code because
__folio_set_anon() would set folio->index to the same value.
Looking at __replace_page(), we'd also pass slightly wrong values to
mmu_notifier_range_init(), page_vma_mapped_walk(), flush_cache_page(),
ptep_clear_flush() and set_pte_at_notify(). I suspect most of them are
fine, but let's just mark the introducing commit as the one needed
fixing. I don't think CC stable is warranted.
We'll add more sanity checks in rmap code separately, to make sure that
we always get properly aligned addresses.
Reported-by: Jiri Olsa <[email protected]>
Closes: https://lkml.kernel.org/r/ZaMR2EWN-HvlCfUl@krava
Fixes: c517ee744b96 ("uprobes: __replace_page() should not use page_address_in_vma()")
Tested-by: Jiri Olsa <[email protected]>
Reviewed-by: Ryan Roberts <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Alexander Shishkin
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Adrian Hunter <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
kernel/events/uprobes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 485bb0389b488..929e98c629652 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -537,7 +537,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
}
}
- ret = __replace_page(vma, vaddr, old_page, new_page);
+ ret = __replace_page(vma, vaddr & PAGE_MASK, old_page, new_page);
if (new_page)
put_page(new_page);
put_old:
--
2.43.0
On 01/15, David Hildenbrand wrote:
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -537,7 +537,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
> }
> }
>
> - ret = __replace_page(vma, vaddr, old_page, new_page);
> + ret = __replace_page(vma, vaddr & PAGE_MASK, old_page, new_page);
Acked-by: Oleg Nesterov <[email protected]>
cc-ing bpf list
jirka
On Mon, Jan 15, 2024 at 11:07:31AM +0100, David Hildenbrand wrote:
> uprobes passes an unaligned page mapping address to
> folio_add_new_anon_rmap(), which ends up triggering a VM_BUG_ON() we
> recently extended in commit 372cbd4d5a066 ("mm: non-pmd-mappable, large
> folios for folio_add_new_anon_rmap()").
>
> Arguably, this is uprobes code doing something wrong; however,
> for the time being it would have likely worked in rmap code because
> __folio_set_anon() would set folio->index to the same value.
>
> Looking at __replace_page(), we'd also pass slightly wrong values to
> mmu_notifier_range_init(), page_vma_mapped_walk(), flush_cache_page(),
> ptep_clear_flush() and set_pte_at_notify(). I suspect most of them are
> fine, but let's just mark the introducing commit as the one needed
> fixing. I don't think CC stable is warranted.
>
> We'll add more sanity checks in rmap code separately, to make sure that
> we always get properly aligned addresses.
>
> Reported-by: Jiri Olsa <[email protected]>
> Closes: https://lkml.kernel.org/r/ZaMR2EWN-HvlCfUl@krava
> Fixes: c517ee744b96 ("uprobes: __replace_page() should not use page_address_in_vma()")
> Tested-by: Jiri Olsa <[email protected]>
> Reviewed-by: Ryan Roberts <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Alexander Shishkin
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Signed-off-by: David Hildenbrand <[email protected]>
> ---
> kernel/events/uprobes.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 485bb0389b488..929e98c629652 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -537,7 +537,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
> }
> }
>
> - ret = __replace_page(vma, vaddr, old_page, new_page);
> + ret = __replace_page(vma, vaddr & PAGE_MASK, old_page, new_page);
> if (new_page)
> put_page(new_page);
> put_old:
> --
> 2.43.0
>
>