Skip to contentSkip to conversation

uprobes: fix kernel info leak via "[uprobes]" vma

PATCH Superseded Oleg Nesterov 2 participants 6 messages Sep 26, 2024 – Sep 29, 2024

Subsystems (2)
perf events uprobes
lore
Share & tools
lore

Oleg Nesterov

author maintainer
#0 lore raw

PATCH 1/1 uprobes: fix kernel info leak via "[uprobes]" vma

xol_add_vma() maps the uninitialized page allocated by __create_xol_area() into userspace. On some architectures (x86) this memory is readable even without VM_READ, VM_EXEC results in the same pgprot_t as VM_EXEC|VM_READ.

Reported-by Will Deacon <[email protected]>
Signed-off-by Oleg Nesterov <[email protected]>
---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/events/uprobes.c +1 -1
index 2ec796e2f055..4b52cb2ae6d6 100644
@@ -1545,7 +1545,7 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
-	area->page = alloc_page(GFP_HIGHUSER);
+	area->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
 	if (!area->page)
 		goto free_bitmap;
Signature · 1 line
2.25.1.362.g51ebf55
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 3 lines · On Thu, 26 Sep 2024 18:29:01 +0200 Oleg Nesterov <[email protected]> wrote:

xol_add_vma() maps the uninitialized page allocated by __create_xol_area() into userspace. On some architectures (x86) this memory is readable even without VM_READ, VM_EXEC results in the same pgprot_t as VM_EXEC|VM_READ.

Good catch!
I think we push this urgently and send it to stable tree too, right?

Fixes d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
Cc [email protected]
Acked-by Masami Hiramatsu (Google) <[email protected]>

Thank you,

Quoted 23 lines
Reported-by Will Deacon <[email protected]>
Signed-off-by Oleg Nesterov <[email protected]>
---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/events/uprobes.c +1 -1
index 2ec796e2f055..4b52cb2ae6d6 100644
@@ -1545,7 +1545,7 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
-	area->page = alloc_page(GFP_HIGHUSER);
+	area->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
 	if (!area->page)
 		goto free_bitmap;
Signature · 1 line
2.25.1.362.g51ebf55
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
author maintainer
#2 lore raw
Quoted 10 lines · Masami Hiramatsu
Quoted 3 lines · On Thu, 26 Sep 2024 18:29:01 +0200 Oleg Nesterov <[email protected]> wrote:

xol_add_vma() maps the uninitialized page allocated by __create_xol_area() into userspace. On some architectures (x86) this memory is readable even without VM_READ, VM_EXEC results in the same pgprot_t as VM_EXEC|VM_READ.

Good catch!
I think we push this urgently and send it to stable tree too, right?

Agreed, can you route this patch?

Quoted 2 lines
Fixes d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
Cc [email protected]

Will add this or should I resend with these tags included ?

perhaps the changelog should mention that debugger can read this memory regardless of pgprot_t...

Quoted 1 line
Acked-by Masami Hiramatsu (Google) <[email protected]>

Thanks!

Oleg.

maintainer
#3 lore raw
Quoted 5 lines · On Sun, 29 Sep 2024 16:50:00 +0200 Oleg Nesterov <[email protected]> wrote:
Quoted 4 lines · Masami Hiramatsu

On Thu, 26 Sep 2024 18:29:01 +0200
Oleg Nesterov <[email protected]> wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

xol_add_vma() maps the uninitialized page allocated by __create_xol_area() into userspace. On some architectures (x86) this memory is readable even without VM_READ, VM_EXEC results in the same pgprot_t as VM_EXEC|VM_READ.

Good catch!
I think we push this urgently and send it to stable tree too, right?

Agreed, can you route this patch?

Yeah, OK.

Quoted 5 lines
Quoted 3 lines
Fixes d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
Cc [email protected]

Will add this or should I resend with these tags included ?

No, but can you confirm this is correct tag? I just track when the alloc_page() came from.

Quoted 3 lines

perhaps the changelog should mention that debugger can read this memory regardless of pgprot_t...

OK, if you can do that, can you add it and resend it?

Thank you,

Quoted 7 lines
Quoted 2 lines
Acked-by Masami Hiramatsu (Google) <[email protected]>

Thanks!

Oleg.

Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
author maintainer
#4 lore raw
Quoted 7 lines · Masami Hiramatsu
Quoted 1 line · On Sun, 29 Sep 2024 16:50:00 +0200 Oleg Nesterov <[email protected]> wrote:

Agreed, can you route this patch?

Yeah, OK.

Great, thank you!

Quoted 1 line

OK, if you can do that, can you add it and resend it?

OK, I'll send V2 with the updated changelog in reply to V1 in a minute.

Thanks,

Oleg.

author maintainer
#5 lore raw

PATCH v2 1/1 uprobes: fix kernel info leak via "[uprobes]" vma

In reply to Oleg Nesterov

xol_add_vma() maps the uninitialized page allocated by __create_xol_area() into userspace. On some architectures (x86) this memory is readable even without VM_READ, VM_EXEC results in the same pgprot_t as VM_EXEC|VM_READ, although this doesn't really matter, debugger can read this memory anyway.

Reported-by Will Deacon <[email protected]>
Fixes d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
Cc [email protected]
Acked-by Masami Hiramatsu (Google) <[email protected]>
Signed-off-by Oleg Nesterov <[email protected]>
---
 kernel/events/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/events/uprobes.c +1 -1
index 2ec796e2f055..4b52cb2ae6d6 100644
@@ -1545,7 +1545,7 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
 	if (!area->bitmap)
 		goto free_area;
 
-	area->page = alloc_page(GFP_HIGHUSER);
+	area->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
 	if (!area->page)
 		goto free_bitmap;
Signature · 1 line
2.25.1.362.g51ebf55