From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jann Horn Subject: Re: [PATCH 1/3] bpf: allow zero-initializing hash map seed Date: Fri, 5 Oct 2018 16:27:58 +0200 Message-ID: References: <20181001104509.24211-1-lmb@cloudflare.com> <20181001104509.24211-2-lmb@cloudflare.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Alexei Starovoitov , Daniel Borkmann , Network Development , Linux API To: lmb@cloudflare.com Return-path: Received: from mail-ot1-f66.google.com ([209.85.210.66]:35395 "EHLO mail-ot1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728621AbeJEV1W (ORCPT ); Fri, 5 Oct 2018 17:27:22 -0400 Received: by mail-ot1-f66.google.com with SMTP id j9-v6so12891343otl.2 for ; Fri, 05 Oct 2018 07:28:25 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Oct 5, 2018 at 4:21 PM Lorenz Bauer wrote: > On Fri, 5 Oct 2018 at 15:12, Jann Horn wrote: > > On Fri, Oct 5, 2018 at 9:42 AM Lorenz Bauer wrote: > > > On Tue, 2 Oct 2018 at 21:00, Jann Horn wrote: > > > > If this is for testing only, you can slap a capable(CAP_SYS_ADMIN) > > > > check in here, right? I doubt it matters, but I don't really like > > > > seeing something like this exposed to unprivileged userspace just > > > > because you need it for kernel testing. > > > > > > That would mean all tests have to run as root / with CAP_SYS_ADMIN > > > which isn't ideal. > > > > This patch basically means that it becomes easier for a local user to > > construct a BPF hash table that has all of its values stuffed into a > > single hash bucket, correct? Which makes it easier to create a BPF > > program that generates unusually large RCU stalls by performing ~40000 > > BPF map lookups, each of which has to walk through the entire linked > > list of the hash map bucket? I dislike exposing something like that to > > unprivileged userspace. > > That's a good point, for which I don't have an answer. You could argue that > this was the status quo until the seed was randomised, so it seems > like this hasn't been a worry so far. Should it be going forward? I don't think that local DoS bugs, or bugs that locally degrade performance, are a big deal, but I also think that the kernel should try to avoid having such issues. > > And if you want to run the whole BPF test suite with all its tests, > > don't you already need root privileges? Or is this a different test > > suite? > > No, I'm thinking about third parties that want to test their own BPF. Ah. That wasn't clear to me from your patch description. Can you please describe exactly why something that is not a kernel unit test needs deterministic BPF hash map behavior? > If you enable unprivileged BPF you can use BPF_PROG_TEST_RUN to > test your programs without root, if I'm not mistaken.