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

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

[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.

Reported-by: Will Deacon <will@kernel.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 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 2ec796e2f055..4b52cb2ae6d6 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 
-- 
2.25.1.362.g51ebf55

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

[Masami Hiramatsu]

On Thu, 26 Sep 2024 18:29:01 +0200
Oleg Nesterov <oleg@redhat.com> 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: stable@vger.kernel.org

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> Reported-by: Will Deacon <will@kernel.org>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  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 2ec796e2f055..4b52cb2ae6d6 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -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;
>  
> -- 
> 2.25.1.362.g51ebf55
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

[Oleg Nesterov]

On 09/29, Masami Hiramatsu wrote:
>
> On Thu, 26 Sep 2024 18:29:01 +0200
> Oleg Nesterov <oleg@redhat.com> 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?

> Fixes: d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
> Cc: stable@vger.kernel.org

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...

> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks!

Oleg.

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

[Masami Hiramatsu]

On Sun, 29 Sep 2024 16:50:00 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 09/29, Masami Hiramatsu wrote:
> >
> > On Thu, 26 Sep 2024 18:29:01 +0200
> > Oleg Nesterov <oleg@redhat.com> 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?

Yeah, OK.

> 
> > Fixes: d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
> > Cc: stable@vger.kernel.org
> 
> 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.

> 
> 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,

> 
> > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Thanks!
> 
> Oleg.
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

[Oleg Nesterov]

On 09/30, Masami Hiramatsu wrote:
>
> On Sun, 29 Sep 2024 16:50:00 +0200
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Agreed, can you route this patch?
>
> Yeah, OK.

Great, thank you!

> 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.

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

[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 <will@kernel.org>
Fixes: d4b3b6384f98 ("uprobes/core: Allocate XOL slots for uprobes use")
Cc: stable@vger.kernel.org
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 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 2ec796e2f055..4b52cb2ae6d6 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -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;
 
-- 
2.25.1.362.g51ebf55