All of lore.kernel.org
 help / color / mirror / Atom feed
From: KP Singh <kpsingh@chromium.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: KP Singh <kpsingh@chromium.org>, bpf <bpf@vger.kernel.org>,
	Andrii Nakryiko <andriin@fb.com>,
	Brendan Jackman <jackmanb@google.com>,
	Florent Revest <revest@google.com>,
	Thomas Garnier <thgarnie@google.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kees Cook <keescook@chromium.org>, Paul Turner <pjt@google.com>,
	Jann Horn <jannh@google.com>,
	Florent Revest <revest@chromium.org>,
	Brendan Jackman <jackmanb@chromium.org>
Subject: Re: [PATCH bpf-next v9 7/8] bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
Date: Thu, 2 Apr 2020 06:03:57 +0200	[thread overview]
Message-ID: <20200402040357.GA217889@google.com> (raw)
In-Reply-To: <CAADnVQKP3mOTUkkzjWM6Qii+v-dCDwV9Ms_-4ptsbdwyDW1MCA@mail.gmail.com>

On 01-Apr 17:09, Alexei Starovoitov wrote:
> On Sat, Mar 28, 2020 at 5:44 PM KP Singh <kpsingh@chromium.org> wrote:
> > +int BPF_PROG(test_int_hook, struct vm_area_struct *vma,
> > +            unsigned long reqprot, unsigned long prot, int ret)
> > +{
> > +       if (ret != 0)
> > +               return ret;
> > +
> > +       __u32 pid = bpf_get_current_pid_tgid() >> 32;
> > +       int is_heap = 0;
> > +
> > +       is_heap = (vma->vm_start >= vma->vm_mm->start_brk &&
> > +                  vma->vm_end <= vma->vm_mm->brk);
> 
> This test fails for me.

Trying this from bpf/master:

  b9258a2cece4 ("slcan: Don't transmit uninitialized stack data in padding")

also from bpf-next/master:

 1a323ea5356e ("x86: get rid of 'errret' argument to __get_user_xyz() macross")

and I am unable to reproduce the failure (the output when using bpf/master):

 ./test_progs -t test_lsm
#70 test_lsm:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 10/10   #P:4
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
            true-322   [001] ...2   187.127231: 0: start 7fc7ffccc000 556a623be000
            true-322   [001] ...2   187.127238: 0: end 7fc7ffe8c000 556a623be000
            true-322   [001] ...2   187.128233: 0: start 7fc7ffe82000 556a623be000
            true-322   [001] ...2   187.128237: 0: end 7fc7ffe88000 556a623be000
            true-322   [001] ...2   187.128306: 0: start 556a604f7000 556a623be000
            true-322   [001] ...2   187.128309: 0: end 556a604f9000 556a623be000
            true-322   [001] ...2   187.128372: 0: start 7fc7ffebf000 556a623be000
            true-322   [001] ...2   187.128375: 0: end 7fc7ffec1000 556a623be000
      test_progs-321   [000] ...2   187.129952: 0: start 55dc6e8df000 55dc6e8df000
      test_progs-321   [000] ...2   187.129955: 0: end 55dc6e906000 55dc6e906000

The full run also works for me:

./test_progs

[...]

#70 test_lsm:OK
#71 test_overhead:OK
#72 tp_attach_query:OK

My config is:

  https://gist.githubusercontent.com/sinkap/cb24b955e1b6e6c1dc736054a774fb41/raw/

and the kernel commandline is (on a QEMU VM):

cat /proc/cmdline
console=ttyS0,115200 root=/dev/sda rw nokaslr

Could you share your config and cmdline?

Also, I am wondering if this happens just in the BPF program or also
in the kernel as the other variable I can think of is the compiled
bpf program itself which might be reading a different value thinking
it's vm->vma_start, possible something to do with BTF / CO RE due to a
compiler bug:

Here's the version of clang I am using:

  clang version 10.0.0 (https://github.com/llvm/llvm-project.git 2026d7b80a1a5534b5e263683c85aa95e7593b98)

- KP

> I've added:
>         bpf_printk("start %llx %llx\n", vma->vm_start, vma->vm_mm->start_brk);
>         bpf_printk("end %llx %llx\n", vma->vm_end, vma->vm_mm->brk);
> and see
> cat /sys/kernel/debug/tracing/trace_pipe
>             true-2285  [001] ...2   858.717432: 0: start 7f66470a2000 607000
>             true-2285  [001] ...2   858.717440: 0: end 7f6647443000 607000
>             true-2285  [001] ...2   858.717658: 0: start 7f6647439000 607000
>             true-2285  [001] ...2   858.717659: 0: end 7f664743f000 607000
>             true-2285  [001] ...2   858.717691: 0: start 605000 607000
>             true-2285  [001] ...2   858.717692: 0: end 607000 607000
>             true-2285  [001] ...2   858.717700: 0: start 7f6647666000 607000
>             true-2285  [001] ...2   858.717701: 0: end 7f6647668000 607000
>       test_progs-2283  [000] ...2   858.718030: 0: start 523000 39b9000
>       test_progs-2283  [000] ...2   858.718033: 0: end 39e0000 39e0000
> 
> 523000 is not >= 39b9000.
> 523000 is higher than vm_mm->end_data, but lower than vm_mm->start_brk.
> No idea why this addr is passed into security_file_mprotect().
> The address user space is passing to mprotect() is 0x39c0000 which is correct.
> Could you please help debug?

  reply	other threads:[~2020-04-02  4:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-29  0:43 [PATCH bpf-next v9 0/8] MAC and Audit policy using eBPF (KRSI) KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 1/8] bpf: Introduce BPF_PROG_TYPE_LSM KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 2/8] security: Refactor declaration of LSM hooks KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 3/8] bpf: lsm: provide attachment points for BPF LSM programs KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 4/8] bpf: lsm: Implement attach, detach and execution KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 5/8] bpf: lsm: Initialize the BPF LSM hooks KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 6/8] tools/libbpf: Add support for BPF_PROG_TYPE_LSM KP Singh
2020-03-29  0:43 ` [PATCH bpf-next v9 7/8] bpf: lsm: Add selftests " KP Singh
2020-04-02  0:09   ` Alexei Starovoitov
2020-04-02  4:03     ` KP Singh [this message]
2020-04-02  4:30       ` Alexei Starovoitov
2020-04-02  5:15         ` Jann Horn
2020-04-02 11:53           ` KP Singh
2020-04-02 14:38             ` Jann Horn
2020-04-02 14:40               ` KP Singh
2020-04-02 15:57             ` Alexei Starovoitov
2020-03-29  0:43 ` [PATCH bpf-next v9 8/8] bpf: lsm: Add Documentation KP Singh
2020-03-29 23:46 ` [PATCH bpf-next v9 0/8] MAC and Audit policy using eBPF (KRSI) Daniel Borkmann
2020-04-29 12:31 ` Mikko Ylinen
2020-04-29 12:34   ` KP Singh
2020-04-29 12:45     ` Mikko Ylinen
2020-04-29 16:17       ` KP Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200402040357.GA217889@google.com \
    --to=kpsingh@chromium.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andriin@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jackmanb@chromium.org \
    --cc=jackmanb@google.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=pjt@google.com \
    --cc=revest@chromium.org \
    --cc=revest@google.com \
    --cc=thgarnie@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.