From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0018FC38145 for ; Thu, 8 Sep 2022 15:17:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231517AbiIHPRe (ORCPT ); Thu, 8 Sep 2022 11:17:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59378 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229789AbiIHPRd (ORCPT ); Thu, 8 Sep 2022 11:17:33 -0400 Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0DAA740571; Thu, 8 Sep 2022 08:17:32 -0700 (PDT) Received: by mail-ej1-x630.google.com with SMTP id go34so3790453ejc.2; Thu, 08 Sep 2022 08:17:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=dvC/u7qJ5dN4X47tbsTGAuz++QpERj+4FDleSIpBEMg=; b=R0hVOMicuPDSnoQmcmYiJB3LFWFfOFwjBgRBuwuOEWI5JTxT51Lu0OjLr++Gxw/lzs Tvik3ezKxyKAnzDTCdQqFNmpHVznLaNacrF9ZuvJc9pBhz0lrAdZi1n+3xp/ijThvgp8 VOwRWcRDrlK9c+JFS1o8ONZvPVH7bR5r3/0ESeh6jy2zLWd7yU9MbottgDeiLnCTWQUN tzuE0V5OqQhzozQQSsV16et/mkj7icGxA9KjWbtIM5r6eFzdxL2e+f9jqf4Nw1JWc8Gp zzjf6KnNbCgGJ8j2doSbs7qY9udYo9nEXzEMNqBxnxO5iPN9tehMBETqPTDu68UbIvgn qldw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=dvC/u7qJ5dN4X47tbsTGAuz++QpERj+4FDleSIpBEMg=; b=bCVLOJe+uWFWhqRYSGlMsFWddY0ccOuslSZ8SxdL5L4VKGycPyhjMCScLRhrL0wnfH RsYzRQaebJgmm78Cg/YaAN4h7x05qFflvpl/48d9F+9zdZvvCyQq7TL4x9K/CZjvJyKp MBqif/DNTnyFdBHskvp2mk1B8zH0UkInMhxX7ly6sfwJuQkpGto1JBRRvrIjiFf3x+97 o5C8f42lClJCCLQaBHcMn+tBo5h3qGNYa5g+kVTJEFlKNVY8QQcOH4CmslmqXyG5fWq1 vvyGtTkV49j8EDzzYtl4fiB78Cu5BoqX0p4ICS6c4nbSy8sIiz0tWrTpps/jPlbQ+W6X NDEg== X-Gm-Message-State: ACgBeo2kHgehmbKYlK/bn9FOqYfXQFkMOZxRq42BTwPpoWaewC3p1ZtZ i4+mAZFTL3LeSGmr2QXgeLi+SG2lq4CZ77J0pa8= X-Google-Smtp-Source: AA6agR6lkwvnPcInorNKbMkhlPy/kqMnwSqf9ilS5C3PIU+zXOi3rmnC/1PYHJgUShzGaAZn2JBWrMwTREu95J9WrlE= X-Received: by 2002:a17:907:a04f:b0:772:da0b:e2f1 with SMTP id gz15-20020a170907a04f00b00772da0be2f1mr3625960ejc.327.1662650250524; Thu, 08 Sep 2022 08:17:30 -0700 (PDT) MIME-Version: 1.0 References: <20220906170301.256206-1-roberto.sassu@huaweicloud.com> <20220906170301.256206-2-roberto.sassu@huaweicloud.com> <02309cfbc1ce47f7de6be8addc2caa315b1fee1b.camel@huaweicloud.com> <8d7a713e500b5e3fce93e4c5c7b8841eb6dd28e4.camel@huaweicloud.com> In-Reply-To: <8d7a713e500b5e3fce93e4c5c7b8841eb6dd28e4.camel@huaweicloud.com> From: Alexei Starovoitov Date: Thu, 8 Sep 2022 08:17:19 -0700 Message-ID: Subject: Re: [PATCH 1/7] bpf: Add missing fd modes check for map iterators To: Roberto Sassu Cc: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Mykola Lysenko , Shuah Khan , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jakub Sitnicki , bpf , Network Development , "open list:KERNEL SELFTEST FRAMEWORK" , "open list:CONTROL GROUP (CGROUP)" , LKML , Hou Tao , Roberto Sassu , stable , Chenbo Feng , LSM List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: On Thu, Sep 8, 2022 at 6:59 AM Roberto Sassu wrote: > > On Wed, 2022-09-07 at 09:02 -0700, Alexei Starovoitov wrote: > > > > [...] > > > > Well, if you write a security module to prevent writes on a map, > > > and > > > user space is able to do it anyway with an iterator, what is the > > > purpose of the security module then? > > > > sounds like a broken "security module" and nothing else. > > Ok, if a custom security module does not convince you, let me make a > small example with SELinux. > > I created a small map iterator that sets every value of a map to 5: > > SEC("iter/bpf_map_elem") > int write_bpf_hash_map(struct bpf_iter__bpf_map_elem *ctx) > { > u32 *key = ctx->key; > u8 *val = ctx->value; > > if (key == NULL || val == NULL) > return 0; > > *val = 5; > return 0; > } > > I create and pin a map: > > # bpftool map create /sys/fs/bpf/map type array key 4 value 1 entries 1 > name test > > Initially, the content of the map looks like: > > # bpftool map dump pinned /sys/fs/bpf/map > key: 00 00 00 00 value: 00 > Found 1 element > > I then created a new SELinux type bpftool_test_t, which has only read > permission on maps: > > # sesearch -A -s bpftool_test_t -t unconfined_t -c bpf > allow bpftool_test_t unconfined_t:bpf map_read; > > So, what I expect is that this type is not able to write to the map. > > Indeed, the current bpftool is not able to do it: > > # strace -f -etrace=bpf runcon -t bpftool_test_t bpftool iter pin > writer.o /sys/fs/bpf/iter map pinned /sys/fs/bpf/map > bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/map", bpf_fd=0, file_flags=0}, > 144) = -1 EACCES (Permission denied) > Error: bpf obj get (/sys/fs/bpf): Permission denied > > This happens because the current bpftool requests to access the map > with read-write permission, and SELinux denies it: > > # cat /var/log/audit/audit.log|audit2allow > > > #============= bpftool_test_t ============== > allow bpftool_test_t unconfined_t:bpf map_write; > > > The command failed, and the content of the map is still: > > # bpftool map dump pinned /sys/fs/bpf/map > key: 00 00 00 00 value: 00 > Found 1 element > > > Now, what I will do is to use a slightly modified version of bpftool > which requests read-only access to the map instead: > > # strace -f -etrace=bpf runcon -t bpftool_test_t ./bpftool iter pin > writer.o /sys/fs/bpf/iter map pinned /sys/fs/bpf/map > bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/map", bpf_fd=0, > file_flags=BPF_F_RDONLY}, 16) = 3 > libbpf: elf: skipping unrecognized data section(5) .eh_frame > libbpf: elf: skipping relo section(6) .rel.eh_frame for section(5) > .eh_frame > > ... > > bpf(BPF_LINK_CREATE, {link_create={prog_fd=4, target_fd=0, > attach_type=BPF_TRACE_ITER, flags=0}, ...}, 48) = 5 > bpf(BPF_OBJ_PIN, {pathname="/sys/fs/bpf/iter", bpf_fd=5, file_flags=0}, > 16) = 0 > > That worked, because SELinux grants read-only permission to > bpftool_test_t. However, the map iterator does not check how the fd was > obtained, and thus allows the iterator to be created. > > At this point, we have write access, despite not having the right to do > it: That is a wrong assumption to begin with. Having an fd to a bpf object (map, link, prog) allows access. read/write sort-of applicable to maps, but not so much to progs, links. That file based read/write flag is only for user processes. bpf progs always had separate flags for that. See BPF_F_RDONLY vs BPF_F_RDONLY_PROG. One doesn't imply the other.