All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [RFC PATCH bpf-next 4/8] bpf: support GET_FD_BY_ID and GET_NEXT_ID for bpf_link
Date: Tue, 14 Apr 2020 12:32:04 +0200	[thread overview]
Message-ID: <87mu7enysb.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzbXCsHCJ6Tet0i5g=pKB_uYqvgiaBNuY-NMdZm8rdZN5g@mail.gmail.com>

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

>> > After that, one can pin bpf_link temporarily and re-open it as
>> > writable one, provided CAP_DAC_OVERRIDE capability is present. All
>> > that works already, because pinned bpf_link is just a file, so one can
>> > do fchmod on it and all that will go through normal file access
>> > permission check code path.
>>
>> Ah, I did not know that was possible - I was assuming that bpffs was
>> doing something special to prevent that. But if not, great!
>>
>> > Unfortunately, just re-opening same FD as writable (which would
>> > be possible if fcntl(fd, F_SETFL, S_IRUSR
>> >  S_IWUSR) was supported on Linux) without pinning is not possible.
>> > Opening link from /proc/<pid>/fd/<link-fd> doesn't seem to work
>> > either, because backing inode is not BPF FS inode. I'm not sure, but
>> > maybe we can support the latter eventually. But either way, I think
>> > given this is to be used for manual troubleshooting, going through few
>> > extra hoops to force-detach bpf_link is actually a good thing.
>>
>> Hmm, I disagree that deliberately making users jump through hoops is a
>> good thing. Smells an awful lot like security through obscurity to me;
>> and we all know how well that works anyway...
>
> Depends on who users are? bpftool can implement this as one of
> `bpftool link` sub-commands and allow human operators to force-detach
> bpf_link, if necessary.

Yeah, I would expect this to be the common way this would be used: built
into tools.

> I think applications shouldn't do this (programmatically) at all,
> which is why I think it's actually good that it's harder and not
> obvious, this will make developer think again before implementing
> this, hopefully. For me it's about discouraging bad practice.

I guess I just don't share your optimism that making people jump through
hoops will actually discourage them :)

If people know what they are doing it should be enough to document it as
discouraged. And if they don't, they are perfectly capable of finding
and copy-pasting the sequence of hoop-jumps required to achieve what
they want, probably with more bugs added along the way.

So in the end I think that all you're really achieving is annoying
people who do have a legitimate reason to override the behaviour (which
includes yourself as a bpftool developer :)). That's what I meant by the
'security through obscurity' comment.

-Toke


  reply	other threads:[~2020-04-14 10:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-04  0:09 [RFC PATCH bpf-next 0/8] bpf_link observability APIs Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 1/8] bpf: refactor bpf_link update handling Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 2/8] bpf: allow bpf_link pinning as read-only and enforce LINK_UPDATE Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 3/8] bpf: allocate ID for bpf_link Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 4/8] bpf: support GET_FD_BY_ID and GET_NEXT_ID " Andrii Nakryiko
2020-04-06 11:34   ` Toke Høiland-Jørgensen
2020-04-06 19:06     ` Andrii Nakryiko
2020-04-08 15:14       ` Toke Høiland-Jørgensen
2020-04-08 20:23         ` Andrii Nakryiko
2020-04-08 21:21           ` Toke Høiland-Jørgensen
2020-04-09 18:49             ` Andrii Nakryiko
2020-04-14 10:32               ` Toke Høiland-Jørgensen [this message]
2020-04-14 18:47                 ` Andrii Nakryiko
2020-04-15  9:26                   ` Toke Høiland-Jørgensen
2020-04-04  0:09 ` [RFC PATCH bpf-next 5/8] bpf: add support for BPF_OBJ_GET_INFO_BY_FD " Andrii Nakryiko
2020-04-06 11:34   ` Toke Høiland-Jørgensen
2020-04-06 18:58     ` Andrii Nakryiko
2020-04-11 10:10   ` kbuild test robot
2020-04-04  0:09 ` [RFC PATCH bpf-next 6/8] libbpf: add low-level APIs for new bpf_link commands Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 7/8] bpftool: expose attach_type-to-string array to non-cgroup code Andrii Nakryiko
2020-04-04  0:09 ` [RFC PATCH bpf-next 8/8] bpftool: add bpf_link show and pin support Andrii Nakryiko
2020-04-08 23:44   ` David Ahern
2020-04-09 18:50     ` Andrii Nakryiko
2020-04-05 16:26 ` [RFC PATCH bpf-next 0/8] bpf_link observability APIs David Ahern
2020-04-05 18:31   ` Andrii Nakryiko

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=87mu7enysb.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --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 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.