lkml.org 
[lkml]   [2021]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH bpf-next] bpf: Allow bpf_d_path in perf_event_mmap
    On Mon, Nov 1, 2021 at 2:17 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
    >
    > Hi,
    >
    >
    > On 2021/10/30 1:02 AM, Florent Revest wrote:
    > > On Fri, Oct 29, 2021 at 12:47 AM Martin KaFai Lau <kafai@fb.com> wrote:
    > >>
    > >> On Thu, Oct 28, 2021 at 06:43:57PM +0200, Florent Revest wrote:
    > >>> Allow the helper to be called from the perf_event_mmap hook. This is
    > >>> convenient to lookup vma->vm_file and implement a similar logic as
    > >>> perf_event_mmap_event in BPF.
    > >> From struct vm_area_struct:
    > >> struct file * vm_file; /* File we map to (can be NULL). */
    > >>
    > >> Under perf_event_mmap, vm_file won't be NULL or bpf_d_path can handle it?
    > >
    > > Thanks Martin, this is a very good point. :) Yes, vm_file can be NULL
    > > in perf_event_mmap.
    > > I wonder what would happen (and what we could do about it? :|).
    > > bpf_d_path is called on &vma->vm_file->f_path So without NULL checks
    > > (of vm_file) in BPF, the helper wouldn't be called with a NULL pointer
    > > but rather with an address that is offsetof(struct file, f_path).
    > >
    >
    > I tested this patch with the following BCC script:
    >
    > bpf_text = '''
    > #include <linux/mm_types.h>
    >
    > KFUNC_PROBE(perf_event_mmap, struct vm_area_struct *vma)
    > {
    > char path[256] = {};
    >
    > bpf_d_path(&vma->vm_file->f_path, path, sizeof(path));
    > bpf_trace_printk("perf_event_mmap %s", path);
    > return 0;
    > }
    > '''
    >
    > b = BPF(text=bpf_text)
    > print("BPF program loaded")
    > b.trace_print()
    >
    > This change causes kernel panic. I think it's because of this NULL pointer.

    Thank you for the testing and repro Hengqi :)
    Indeed, I was able to reproduce this panic. When vma->vm_file is NULL,
    &vma->vm_file->f_path ends up being 0x18 so d_path causes a panic.
    I suppose that this sort of issue must be relatively common in helpers
    that take a PTR_TO_BTF_ID though ? I wonder if there is anything that
    the verifier could do about this ? For example if vma->vm_file could
    be PTR_TO_BTF_ID_OR_NULL and therefore vma->vm_file->f_path somehow
    considered invalid ?

    \
     
     \ /
      Last update: 2021-11-01 16:01    [W:3.668 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site