[bpf] bpf: fix panic in stack_map_get_build_id() on i386 and arm32
diff mbox series

Message ID 20190108222044.4140262-1-songliubraving@fb.com
State Accepted
Delegated to: BPF Maintainers
Headers show
Series
  • [bpf] bpf: fix panic in stack_map_get_build_id() on i386 and arm32
Related show

Commit Message

Song Liu Jan. 8, 2019, 10:20 p.m. UTC
As Naresh reported, test_stacktrace_build_id() causes panic on i386 and
arm32 systems. This is caused by page_address() returns NULL in certain
cases.

This patch fixes this error by using kmap_atomic/kunmap_atomic instead
of page_address.

Fixes: 615755a77b24 (" bpf: extend stackmap to save binary_build_id+offset instead of address")
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 kernel/bpf/stackmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Song Liu Jan. 8, 2019, 10:23 p.m. UTC | #1
> On Jan 8, 2019, at 2:20 PM, Song Liu <songliubraving@fb.com> wrote:
> 
> As Naresh reported, test_stacktrace_build_id() causes panic on i386 and
> arm32 systems. This is caused by page_address() returns NULL in certain
> cases.
> 
> This patch fixes this error by using kmap_atomic/kunmap_atomic instead
> of page_address.
> 
> Fixes: 615755a77b24 (" bpf: extend stackmap to save binary_build_id+offset instead of address")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> kernel/bpf/stackmap.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 90daf285de03..d9e2483669d0 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -260,7 +260,7 @@ static int stack_map_get_build_id(struct vm_area_struct *vma,
> 		return -EFAULT;	/* page not mapped */
> 
> 	ret = -EINVAL;
> -	page_addr = page_address(page);
> +	page_addr = kmap_atomic(page);
> 	ehdr = (Elf32_Ehdr *)page_addr;
> 
> 	/* compare magic x7f "ELF" */
> @@ -276,6 +276,7 @@ static int stack_map_get_build_id(struct vm_area_struct *vma,
> 	else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
> 		ret = stack_map_get_build_id_64(page_addr, build_id);
> out:
> +	kunmap_atomic(page_addr);
> 	put_page(page);
> 	return ret;
> }
> -- 
> 2.17.1
> 

Hi Naresh,

I only tested this patch on i386 vm. Could you please help test it on
ARM32 systems?

Thanks,
Song
Daniel Borkmann Jan. 10, 2019, 3:26 p.m. UTC | #2
On 01/08/2019 11:20 PM, Song Liu wrote:
> As Naresh reported, test_stacktrace_build_id() causes panic on i386 and
> arm32 systems. This is caused by page_address() returns NULL in certain
> cases.
> 
> This patch fixes this error by using kmap_atomic/kunmap_atomic instead
> of page_address.
> 
> Fixes: 615755a77b24 (" bpf: extend stackmap to save binary_build_id+offset instead of address")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Song Liu <songliubraving@fb.com>

Looks correct to me, and in this case as per API the assumption is allowed
that k[un]map_atomic() won't fail. I've applied it to bpf, if Naresh still
replies with a Tested-by in time, I'll add it as well to the commit.
Naresh Kamboju Jan. 17, 2019, 5:24 p.m. UTC | #3
On Thu, 10 Jan 2019 at 20:56, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 01/08/2019 11:20 PM, Song Liu wrote:
> > As Naresh reported, test_stacktrace_build_id() causes panic on i386 and
> > arm32 systems. This is caused by page_address() returns NULL in certain
> > cases.
> >
> > This patch fixes this error by using kmap_atomic/kunmap_atomic instead
> > of page_address.
> >
> > Fixes: 615755a77b24 (" bpf: extend stackmap to save binary_build_id+offset instead of address")
> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> > Signed-off-by: Song Liu <songliubraving@fb.com>

Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>

>
> Looks correct to me, and in this case as per API the assumption is allowed
> that k[un]map_atomic() won't fail. I've applied it to bpf, if Naresh still
> replies with a Tested-by in time, I'll add it as well to the commit.

This patch fixes the problem on Linux kernel 5.0.0-rc2.
Thank you.

Best regards
Naresh Kamboju

Patch
diff mbox series

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 90daf285de03..d9e2483669d0 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -260,7 +260,7 @@  static int stack_map_get_build_id(struct vm_area_struct *vma,
 		return -EFAULT;	/* page not mapped */
 
 	ret = -EINVAL;
-	page_addr = page_address(page);
+	page_addr = kmap_atomic(page);
 	ehdr = (Elf32_Ehdr *)page_addr;
 
 	/* compare magic x7f "ELF" */
@@ -276,6 +276,7 @@  static int stack_map_get_build_id(struct vm_area_struct *vma,
 	else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
 		ret = stack_map_get_build_id_64(page_addr, build_id);
 out:
+	kunmap_atomic(page_addr);
 	put_page(page);
 	return ret;
 }