diff mbox series

[v13,13/20] bpf, arm64: untag user pointers in stack_map_get_build_id_offset

Message ID 09d6b8e5c8275de85c7aba716578fbcb3cbce924.1553093421.git.andreyknvl@google.com (mailing list archive)
State New, archived
Headers show
Series arm64: untag user pointers passed to the kernel | expand

Commit Message

Andrey Konovalov March 20, 2019, 2:51 p.m. UTC
This patch is a part of a series that extends arm64 kernel ABI to allow to
pass tagged user pointers (with the top byte set to something else other
than 0x00) as syscall arguments.

stack_map_get_build_id_offset() uses provided user pointers for vma
lookups, which can only by done with untagged pointers.

Untag user pointers in this function for doing the lookup and
calculating the offset, but save as is in the bpf_stack_build_id
struct.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 kernel/bpf/stackmap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Catalin Marinas March 22, 2019, 3:52 p.m. UTC | #1
On Wed, Mar 20, 2019 at 03:51:27PM +0100, Andrey Konovalov wrote:
> This patch is a part of a series that extends arm64 kernel ABI to allow to
> pass tagged user pointers (with the top byte set to something else other
> than 0x00) as syscall arguments.
> 
> stack_map_get_build_id_offset() uses provided user pointers for vma
> lookups, which can only by done with untagged pointers.
> 
> Untag user pointers in this function for doing the lookup and
> calculating the offset, but save as is in the bpf_stack_build_id
> struct.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  kernel/bpf/stackmap.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 950ab2f28922..bb89341d3faf 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -320,7 +320,9 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>  	}
>  
>  	for (i = 0; i < trace_nr; i++) {
> -		vma = find_vma(current->mm, ips[i]);
> +		u64 untagged_ip = untagged_addr(ips[i]);
> +
> +		vma = find_vma(current->mm, untagged_ip);
>  		if (!vma || stack_map_get_build_id(vma, id_offs[i].build_id)) {
>  			/* per entry fall back to ips */
>  			id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> @@ -328,7 +330,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
>  			memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
>  			continue;
>  		}
> -		id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ips[i]
> +		id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + untagged_ip
>  			- vma->vm_start;
>  		id_offs[i].status = BPF_STACK_BUILD_ID_VALID;
>  	}

Can the ips[*] here ever be tagged?
Andrey Konovalov April 1, 2019, 4 p.m. UTC | #2
On Fri, Mar 22, 2019 at 4:52 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Wed, Mar 20, 2019 at 03:51:27PM +0100, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > stack_map_get_build_id_offset() uses provided user pointers for vma
> > lookups, which can only by done with untagged pointers.
> >
> > Untag user pointers in this function for doing the lookup and
> > calculating the offset, but save as is in the bpf_stack_build_id
> > struct.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> >  kernel/bpf/stackmap.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index 950ab2f28922..bb89341d3faf 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -320,7 +320,9 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
> >       }
> >
> >       for (i = 0; i < trace_nr; i++) {
> > -             vma = find_vma(current->mm, ips[i]);
> > +             u64 untagged_ip = untagged_addr(ips[i]);
> > +
> > +             vma = find_vma(current->mm, untagged_ip);
> >               if (!vma || stack_map_get_build_id(vma, id_offs[i].build_id)) {
> >                       /* per entry fall back to ips */
> >                       id_offs[i].status = BPF_STACK_BUILD_ID_IP;
> > @@ -328,7 +330,7 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
> >                       memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
> >                       continue;
> >               }
> > -             id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ips[i]
> > +             id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + untagged_ip
> >                       - vma->vm_start;
> >               id_offs[i].status = BPF_STACK_BUILD_ID_VALID;
> >       }
>
> Can the ips[*] here ever be tagged?

Those are instruction pointers AFAIU, so no, not within the current
ABI. I'll drop this patch. Thanks!

>
> --
> Catalin
diff mbox series

Patch

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 950ab2f28922..bb89341d3faf 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -320,7 +320,9 @@  static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 	}
 
 	for (i = 0; i < trace_nr; i++) {
-		vma = find_vma(current->mm, ips[i]);
+		u64 untagged_ip = untagged_addr(ips[i]);
+
+		vma = find_vma(current->mm, untagged_ip);
 		if (!vma || stack_map_get_build_id(vma, id_offs[i].build_id)) {
 			/* per entry fall back to ips */
 			id_offs[i].status = BPF_STACK_BUILD_ID_IP;
@@ -328,7 +330,7 @@  static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
 			memset(id_offs[i].build_id, 0, BPF_BUILD_ID_SIZE);
 			continue;
 		}
-		id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + ips[i]
+		id_offs[i].offset = (vma->vm_pgoff << PAGE_SHIFT) + untagged_ip
 			- vma->vm_start;
 		id_offs[i].status = BPF_STACK_BUILD_ID_VALID;
 	}