[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzZtJo5dKMX_ys_2rN+bx6QqDGz9DAEVFod6Ys9Rs93VgA@mail.gmail.com>
Date: Tue, 14 Apr 2020 11:47:37 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
Networking <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Kernel Team <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 4/8] bpf: support GET_FD_BY_ID and
GET_NEXT_ID for bpf_link
On Tue, Apr 14, 2020 at 3:32 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@...il.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 :)
I understand. I just don't see why would anyone have to implement this
at all and especially would think it's a good idea to begin with?
>
> 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.
Can I please get a list of real examples of legitimate reasons to
override this behavior?
>
> -Toke
>
Powered by blists - more mailing lists