From: Sargun Dhillon <sargun@sargun.me>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v4 1/2] bpf: Add bpf_probe_write BPF helper to be called in tracers (kprobes)
Date: Sat, 23 Jul 2016 17:39:42 -0700 [thread overview]
Message-ID: <20160724003941.GA31582@ircssh.c.rugged-nimbus-611.internal> (raw)
In-Reply-To: <20160723193510.GA23128@ast-mbp>
On Sat, Jul 23, 2016 at 12:35:12PM -0700, Alexei Starovoitov wrote:
> On Fri, Jul 22, 2016 at 05:05:27PM -0700, Sargun Dhillon wrote:
> > It was tested with the tracex7 program on x86-64.
>
> it's my fault to start tracexN tradition that turned out to be
> cumbersome, let's not continue it. Instead could you rename it
> to something meaningful? Like test_probe_write_user ?
> Right now it just prints client's peer address and human needs to
> visually verify that probe_write_user actually happened, if you can
> convert it into a test it will help a lot.
> We were planning to convert all of the samples/bpf/ into tests,
> so we can run them continuously.
The example has been modified to act like a test in the follow up set. It tests
for the positive case (Did the helper work or not) as opposed to the negative
case (is the helper able to violate the safety constraints we set forth)? I
could do that as well, in another patch by mprotecting those pages, or some
such. Should I add an additional negative test?
>
> btw, single patch re-submit will not be picked up. Please always
> re-submit the whole patch set together.
>
I'll resubmit the set on another thread. It's just easier doing the reviews
inline.
> > +static const struct bpf_func_proto *bpf_get_probe_write_proto(void) {
> > + pr_warn_once("*****************************************************\n");
> > + pr_warn_once("* bpf_probe_write_user: Experimental Feature in use *\n");
> > + pr_warn_once("* bpf_probe_write_user: Feature may corrupt memory *\n");
> > + pr_warn_once("*****************************************************\n");
> > + pr_notice_ratelimited("bpf_probe_write_user: %s[%d] installing program with helper: it may corrupt user memory!",
> > + current->comm, task_pid_nr(current));
>
> I thought we were argeeing on single pr_warn_ratelimited without banner ?
I'll switch to that.
>
> The rest looks good.
> Thanks!
>
next prev parent reply other threads:[~2016-07-24 0:39 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-22 1:09 [PATCH v4 1/2] bpf: Add bpf_probe_write BPF helper to be called in tracers (kprobes) Sargun Dhillon
2016-07-22 2:14 ` Alexei Starovoitov
2016-07-22 9:53 ` Daniel Borkmann
2016-07-22 15:59 ` Alexei Starovoitov
2016-07-23 0:05 ` Sargun Dhillon
2016-07-23 19:35 ` Alexei Starovoitov
2016-07-24 0:39 ` Sargun Dhillon [this message]
2016-07-24 1:19 ` Alexei Starovoitov
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=20160724003941.GA31582@ircssh.c.rugged-nimbus-611.internal \
--to=sargun@sargun.me \
--cc=alexei.starovoitov@gmail.com \
--cc=daniel@iogearbox.net \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).