From patchwork Wed Jun 16 22:55:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 12326157 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34B12C48BE5 for ; Wed, 16 Jun 2021 22:55:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 11FE8613CD for ; Wed, 16 Jun 2021 22:55:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230334AbhFPW5X (ORCPT ); Wed, 16 Jun 2021 18:57:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230103AbhFPW5V (ORCPT ); Wed, 16 Jun 2021 18:57:21 -0400 Received: from mail-il1-x12c.google.com (mail-il1-x12c.google.com [IPv6:2607:f8b0:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7BE03C061574 for ; Wed, 16 Jun 2021 15:55:14 -0700 (PDT) Received: by mail-il1-x12c.google.com with SMTP id q18so3735958ile.10 for ; Wed, 16 Jun 2021 15:55:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=MYzev/2ejIlEofGZAN0KAN+etWMF72Vq4LTXFhnj74M=; b=fnYjR7kIS+GEby0Bwyp11ZxdK9yhNvBM4o/p7i5FU+jrY8hjsbba2hNlJwiQkPjV/x 0MoFXqqKsZW58PkRXVs4XveaU5eclBQDb/4kSKI9QHFOqmDHMCi34xKWoN6VyrErYjpb Blzq9l3yM/ePtL4dflbD9nwNCoITHU7/Ycv1bO66DE3LFZ6bopusklaOsrRcAvu6K7Xj LskkGhliu+cArzQWHGEgIKSUY5Tc83apFhRE+GQUe10oCf6Bbe1KluJWZYXuYFX59/Uu RxyRm0njIRGX4iV7wrcJ1HrKeT8Kf0JQymgjLJwimYjXTVe/OKou+/Sa/jRYWc8aOrAZ Y/Ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=MYzev/2ejIlEofGZAN0KAN+etWMF72Vq4LTXFhnj74M=; b=Z7dkMPF9f1ywWD2YqtCgjcfPV5s3Gd7NCa736c3rMlicNJ4V/5sRRnGfc6AjKTO/53 Ag6RMO2SxG7Q96FjqnCZQop/rPGaWr49TZ4YbsbJkmbokuANyeeRCajoxqsBJJ1GCLBY HR+1OTrNjmc79Ny91Lmv1DcD/eTC+LAQJuGYjVE9e74evI5kpyvp6TBBJtH5585h6BcA QWNt9FNv7ZWgYTm9aEtT9uSYIAaTkef6e1baALzMnAtzM9LjQY2TMfpdK/r4cmVfvRPH Jkq9PYXNN/zZ1iqJz+Z9WOxINa9wU/tWRSoCGo+4uF6iZfGfw8SiZk1/KBedpLX+UzrG sdkQ== X-Gm-Message-State: AOAM531q5eiA5lXfOM8kzfbGvg/RqgG3OsWEaLU1fQASruEPnBnkXvW2 hB7DYq8YM0i0WQYvLlu5wEg= X-Google-Smtp-Source: ABdhPJyrTptKn7IWXK+sXTwhY2ecuMfBtxI1N/r/oVZhM/7361LIAq0x0fZ3ABbLTa/AWHiIn+PCtQ== X-Received: by 2002:a05:6e02:1606:: with SMTP id t6mr1349473ilu.44.1623884113854; Wed, 16 Jun 2021 15:55:13 -0700 (PDT) Received: from [127.0.1.1] ([172.243.157.240]) by smtp.gmail.com with ESMTPSA id h26sm1859747ioh.34.2021.06.16.15.55.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jun 2021 15:55:13 -0700 (PDT) Subject: [PATCH bpf v2 1/4] bpf: Fix null ptr deref with mixed tail calls and subprogs From: John Fastabend To: maciej.fijalkowski@intel.com, ast@kernel.org, daniel@iogearbox.net, andriin@fb.com Cc: john.fastabend@gmail.com, netdev@vger.kernel.org, netdev@vger.kernel.org Date: Wed, 16 Jun 2021 15:55:00 -0700 Message-ID: <162388410082.151936.6522658624805533014.stgit@john-XPS-13-9370> In-Reply-To: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> References: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> User-Agent: StGit/0.23-85-g6af9 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net The sub-programs prog->aux->poke_tab[] is populated in jit_subprogs() and then used when emitting 'BPF_JMP|BPF_TAIL_CALL' insn->code from the individual JITs. The poke_tab[] to use is stored in the insn->imm by the code adding it to that array slot. The JIT then uses imm to find the right entry for an individual instruction. In the x86 bpf_jit_comp.c this is done by calling emit_bpf_tail_call_direct with the poke_tab[] of the imm value. However, we observed the below null-ptr-deref when mixing tail call programs with subprog programs. For this to happen we just need to mix bpf-2-bpf calls and tailcalls with some extra calls or instructions that would be patched later by one of the fixup routines. So whats happening? Before the fixup_call_args() -- where the jit op is done -- various code patching is done by do_misc_fixups(). This may increase the insn count, for example when we patch map_lookup_up using map_gen_lookup hook. This does two things. First, it means the instruction index, insn_idx field, of a tail call instruction will move by a 'delta'. In verifier code, struct bpf_jit_poke_descriptor desc = { .reason = BPF_POKE_REASON_TAIL_CALL, .tail_call.map = BPF_MAP_PTR(aux->map_ptr_state), .tail_call.key = bpf_map_key_immediate(aux), .insn_idx = i + delta, }; Then subprog start values subprog_info[i].start will be updated with the delta and any poke descriptor index will also be updated with the delta in adjust_poke_desc(). If we look at the adjust subprog starts though we see its only adjusted when the delta occurs before the new instructions, /* NOTE: fake 'exit' subprog should be updated as well. */ for (i = 0; i <= env->subprog_cnt; i++) { if (env->subprog_info[i].start <= off) continue; Earlier subprograms are not changed because their start values are not moved. But, adjust_poke_desc() does the offset + delta indiscriminately. The result is poke descriptors are potentially corrupted. Then in jit_subprogs() we only populate the poke_tab[] when the above insn_idx is less than the next subprogram start. From above we corrupted our insn_idx so we might incorrectly assume a poke descriptor is not used in a subprogram omitting it from the subprogram. And finally when the jit runs it does the deref of poke_tab when emitting the instruction and crashes with below. Because earlier step omitted the poke descriptor. The fix is straight forward with above context. Simply move same logic from adjust_subprog_starts() into adjust_poke_descs() and only adjust insn_idx when needed. [ 82.396354] bpf_testmod: version magic '5.12.0-rc2alu+ SMP preempt mod_unload ' should be '5.12.0+ SMP preempt mod_unload ' [ 82.623001] loop10: detected capacity change from 0 to 8 [ 88.487424] ================================================================== [ 88.487438] BUG: KASAN: null-ptr-deref in do_jit+0x184a/0x3290 [ 88.487455] Write of size 8 at addr 0000000000000008 by task test_progs/5295 [ 88.487471] CPU: 7 PID: 5295 Comm: test_progs Tainted: G I 5.12.0+ #386 [ 88.487483] Hardware name: Dell Inc. Precision 5820 Tower/002KVM, BIOS 1.9.2 01/24/2019 [ 88.487490] Call Trace: [ 88.487498] dump_stack+0x93/0xc2 [ 88.487515] kasan_report.cold+0x5f/0xd8 [ 88.487530] ? do_jit+0x184a/0x3290 [ 88.487542] do_jit+0x184a/0x3290 ... [ 88.487709] bpf_int_jit_compile+0x248/0x810 ... [ 88.487765] bpf_check+0x3718/0x5140 ... [ 88.487920] bpf_prog_load+0xa22/0xf10 Reported-by: Jussi Maki CC: Maciej Fijalkowski Fixes: a748c6975dea3 ("bpf: propagate poke descriptors to subprograms") Reviewed-by: Daniel Borkmann Signed-off-by: John Fastabend --- kernel/bpf/verifier.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c6a27574242d..6e2ebcb0d66f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11459,7 +11459,7 @@ static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len } } -static void adjust_poke_descs(struct bpf_prog *prog, u32 len) +static void adjust_poke_descs(struct bpf_prog *prog, u32 off, u32 len) { struct bpf_jit_poke_descriptor *tab = prog->aux->poke_tab; int i, sz = prog->aux->size_poke_tab; @@ -11467,6 +11467,8 @@ static void adjust_poke_descs(struct bpf_prog *prog, u32 len) for (i = 0; i < sz; i++) { desc = &tab[i]; + if (desc->insn_idx <= off) + continue; desc->insn_idx += len - 1; } } @@ -11487,7 +11489,7 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of if (adjust_insn_aux_data(env, new_prog, off, len)) return NULL; adjust_subprog_starts(env, off, len); - adjust_poke_descs(new_prog, len); + adjust_poke_descs(new_prog, off, len); return new_prog; } From patchwork Wed Jun 16 22:55:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 12326159 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A963DC48BE6 for ; Wed, 16 Jun 2021 22:55:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 95770613CD for ; Wed, 16 Jun 2021 22:55:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230352AbhFPW5l (ORCPT ); Wed, 16 Jun 2021 18:57:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53674 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229602AbhFPW5k (ORCPT ); Wed, 16 Jun 2021 18:57:40 -0400 Received: from mail-io1-xd2f.google.com (mail-io1-xd2f.google.com [IPv6:2607:f8b0:4864:20::d2f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4B296C061574 for ; Wed, 16 Jun 2021 15:55:34 -0700 (PDT) Received: by mail-io1-xd2f.google.com with SMTP id s26so962604ioe.9 for ; Wed, 16 Jun 2021 15:55:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=KgcDupNr6WzckA/mD3zDMlYk7N37gqCIpQ3Xwu0Qvtc=; b=DCHWGrfhDE7QG2ybVwvdLsjflOHljDqYk/q3y9A6pUkHQ4wdy9inFN3ugjG8Fxt6zU 4RNzWEhUfxBE4c5u9XxUiwbSyQobBgSZksF7RkyIcm8DOil0zL0SoWcV5N5uEFpCuoJx 8F3K3NIJk9kwAva+s9IHnuqWfcMlheM1gGibrEt9r/vKf73uR/26V8iU1aiUNLMWa7MH k8yixSUHg8TqYr5ALQUiQxQD+gfJA0QuOPNxhn6NXvco45Bc6qp3qEog2HKTPE10ZdZk OXTSmpviqwAG8C/ddvXzk5CDjPLWu+C1PtVx97MQxeU/uUF8kXhraObCzqMayjxmW0U9 YE/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=KgcDupNr6WzckA/mD3zDMlYk7N37gqCIpQ3Xwu0Qvtc=; b=ShsbwudCBPRWaBxjPSjPryqhKFbQJukbFcBZL+nqWMS9C8+tEG/INdKq6I1BuXmJ7U 8su/loHUlNPG/QQXh7njUKhI3JhlBknKaJtub/Xp8EvIOF3d2fhh+psXL2HlvF1e7/MM 4OwRzlOVgOs0Z3x8kxDM3PMRka3CNxp0GZ7Ubdt4nSl755mqR+P2LCxiEkb4CFKjs17A RrSrZsQC0LW8IdG9zTFbXF2zzAra2Ey+BNf7J/kpfec7lRA3sRjWu4GSStUg5FNVEN5P nhmuXbdyHPUFtHkqqnxmrVgaAtWTIRp8iOyaSxKJO27yxcMYgfpkw36nXB0qxMDuqDWy j0Gg== X-Gm-Message-State: AOAM533cSnsGRs7N8Z2noLj9I/CkRZYMRrxFOnivb7aTh2lrnoR/NUEH GuyxFJmnfTu8bL61KWSfGZw= X-Google-Smtp-Source: ABdhPJxNazkXRdv4QQyBCHnj3N4TKx0Dj8NSwwNKV5JSbe4zxk5T2tvld3bmsGo0tRcfiHAhc9nVlw== X-Received: by 2002:a6b:e40a:: with SMTP id u10mr1336978iog.200.1623884133700; Wed, 16 Jun 2021 15:55:33 -0700 (PDT) Received: from [127.0.1.1] ([172.243.157.240]) by smtp.gmail.com with ESMTPSA id z14sm1827639ilb.48.2021.06.16.15.55.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jun 2021 15:55:33 -0700 (PDT) Subject: [PATCH bpf v2 2/4] bpf: map_poke_descriptor is being called with an unstable poke_tab[] From: John Fastabend To: maciej.fijalkowski@intel.com, ast@kernel.org, daniel@iogearbox.net, andriin@fb.com Cc: john.fastabend@gmail.com, netdev@vger.kernel.org, netdev@vger.kernel.org Date: Wed, 16 Jun 2021 15:55:19 -0700 Message-ID: <162388411986.151936.3914295553899556046.stgit@john-XPS-13-9370> In-Reply-To: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> References: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> User-Agent: StGit/0.23-85-g6af9 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net When populating poke_tab[] of a subprog we call map_poke_track() after doing bpf_jit_add_poke_descriptor(). But, bpf_jit_add_poke_descriptor() may, likely will, realloc the poke_tab[] structure and free the old one. So that prog->aux->poke_tab is not stable. However, the aux pointer is referenced from bpf_array_aux and poke_tab[] is used to 'track' prog<->map link. This way when progs are released the entry in the map is dropped and vice versa when the map is released we don't drop it too soon if a prog is in the process of calling it. I wasn't able to trigger any errors here, for example having map_poke_run run with a poke_tab[] pointer that was free'd from bpf_jit_add_poke_descriptor(), but it looks possible and at very least is very fragile. This patch moves poke_track call out of loop that is calling add_poke so that we only ever add stable aux->poke_tab pointers to the map's bpf_array_aux struct. Further, we need this in the next patch to fix a real bug where progs are not 'untracked'. Signed-off-by: John Fastabend --- kernel/bpf/verifier.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6e2ebcb0d66f..066fac9b5460 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -12126,8 +12126,12 @@ static int jit_subprogs(struct bpf_verifier_env *env) } func[i]->insnsi[insn_idx - subprog_start].imm = ret + 1; + } - map_ptr = func[i]->aux->poke_tab[ret].tail_call.map; + for (j = 0; j < func[i]->aux->size_poke_tab; j++) { + int ret; + + map_ptr = func[i]->aux->poke_tab[j].tail_call.map; ret = map_ptr->ops->map_poke_track(map_ptr, func[i]->aux); if (ret < 0) { verbose(env, "tracking tail call prog failed\n"); From patchwork Wed Jun 16 22:55:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 12326161 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CE95FC48BE5 for ; Wed, 16 Jun 2021 22:55:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A653B613CB for ; Wed, 16 Jun 2021 22:55:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231249AbhFPW6A (ORCPT ); Wed, 16 Jun 2021 18:58:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53748 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230352AbhFPW57 (ORCPT ); Wed, 16 Jun 2021 18:57:59 -0400 Received: from mail-il1-x12e.google.com (mail-il1-x12e.google.com [IPv6:2607:f8b0:4864:20::12e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B9453C061574 for ; Wed, 16 Jun 2021 15:55:52 -0700 (PDT) Received: by mail-il1-x12e.google.com with SMTP id j14so3749156ila.6 for ; Wed, 16 Jun 2021 15:55:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=T8qbS7NbckTl+8pauuGnPZDRe2QMX0EMkKMWVI6/vyc=; b=piByfiAVktCUeODPguQVOgLWA9WD+W0MbMOxU4n8H5mviLVsEGs+9+IJdy+97kvgbY HZE5t0I19EsuZ/K5d+EficyShv9lz8X8BjEpOS7NMC8m9/KNavkNFwqkXyyAdy3tX0LU +1lI5wPVVYYMZyBPJJcxQ50T9vkHOxlcQuAa3h7Cwsd0jdpucOip00zvTR0qzuqlGqc7 6D5UHJ1MsHiKyZQBULa+Y15pK+YU5A4IbBr67JOxOGtxnHPP+OS9R3mQ8oScZ+CMBslS EVEePBHUcbIXfOd8e+4CkBp/zBzAX6tg6hf7DftSJ597WE6suBY6HlMYUYk+4r0ornH6 NOqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=T8qbS7NbckTl+8pauuGnPZDRe2QMX0EMkKMWVI6/vyc=; b=ldDK2J+uOxodiiyOEGbNkCKY7sBlt9RHp9Lk8KYbo9OYkjaycjgae3O4rxW0wbdjIE sFBSRj8uQuZZiHgVyWMkEBPqPbjrOpQda20Cuusgf1G3BeZpoN7HTpTMhj/psAHbHyos AgCn7waJot1SY/EKG5GfbxJ7+I+vezr7LrChmzpYhLlUAPbd9dD4OdEqLCFS4vzf9YqI Ksk5ahcuDzZIxs3eAIdh2B4xlrVMyyH4P6mcy4qIkfn16xIJlRo92YJu7EoVBStJUy8V nIC+lIcO5S/qFUipIVp405G0yZI2nXTl+2KFIObYDde7OuFR1DJd1o8uVvX+Ge6RiGfL r+jw== X-Gm-Message-State: AOAM530TsABh98XVNvWKdvz7C3ryj5DCH6I4oIuBhzuYzoUHA30hs4nW 3vEFYqsYFY07I0KtrFFHCsg= X-Google-Smtp-Source: ABdhPJxdrQvGGht9Z9qO+NZeMsxVm7h7v57xtwzmnrKCS34jLknRwSd4FhQoyIE8n8DQAvBTc621UQ== X-Received: by 2002:a92:c0c9:: with SMTP id t9mr1321269ilf.195.1623884151593; Wed, 16 Jun 2021 15:55:51 -0700 (PDT) Received: from [127.0.1.1] ([172.243.157.240]) by smtp.gmail.com with ESMTPSA id 6sm2105581ioe.43.2021.06.16.15.55.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jun 2021 15:55:51 -0700 (PDT) Subject: [PATCH bpf v2 3/4] bpf: track subprog poke correctly From: John Fastabend To: maciej.fijalkowski@intel.com, ast@kernel.org, daniel@iogearbox.net, andriin@fb.com Cc: john.fastabend@gmail.com, netdev@vger.kernel.org, netdev@vger.kernel.org Date: Wed, 16 Jun 2021 15:55:39 -0700 Message-ID: <162388413965.151936.16775592753297385087.stgit@john-XPS-13-9370> In-Reply-To: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> References: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> User-Agent: StGit/0.23-85-g6af9 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net Subprograms are calling map_poke_track but on program release there is no hook to call map_poke_untrack. But on prog release the aux memory is freed even though we still have a reference to it in the element list of the map aux data. So when we run map_poke_run() we end up accessing free'd memory. This triggers with KASAN in prog_array_map_poke_run() shown here. [ 402.824686] ================================================================== [ 402.824689] BUG: KASAN: use-after-free in prog_array_map_poke_run+0xc2/0x34e [ 402.824698] Read of size 4 at addr ffff8881905a7940 by task hubble-fgs/4337 [ 402.824705] CPU: 1 PID: 4337 Comm: hubble-fgs Tainted: G I 5.12.0+ #399 [ 402.824715] Call Trace: [ 402.824719] dump_stack+0x93/0xc2 [ 402.824727] print_address_description.constprop.0+0x1a/0x140 [ 402.824736] ? prog_array_map_poke_run+0xc2/0x34e [ 402.824740] ? prog_array_map_poke_run+0xc2/0x34e [ 402.824744] kasan_report.cold+0x7c/0xd8 [ 402.824752] ? prog_array_map_poke_run+0xc2/0x34e [ 402.824757] prog_array_map_poke_run+0xc2/0x34e [ 402.824765] bpf_fd_array_map_update_elem+0x124/0x1a0 The elements concerned are walked like this, for (i = 0; i < elem->aux->size_poke_tab; i++) { poke = &elem->aux->poke_tab[i]; So the access to size_poke_tab is the 4B read, verified by checking offsets in the KASAN dump, [ 402.825004] The buggy address belongs to the object at ffff8881905a7800 which belongs to the cache kmalloc-1k of size 1024 [ 402.825008] The buggy address is located 320 bytes inside of 1024-byte region [ffff8881905a7800, ffff8881905a7c00) With pahol output, struct bpf_prog_aux { ... /* --- cacheline 5 boundary (320 bytes) --- */ u32 size_poke_tab; /* 320 4 */ ... To fix track the map references by using a per subprogram used_maps array and used_map_cnt values to hold references into the maps so when the subprogram is released we can then untrack from the correct map using the correct aux field. Here we a slightly less than optimal because we insert all poke entries into the used_map array, even duplicates. In theory we could get by with only unique entries. This would require an extra collect the maps stage though and seems unnecessary when this is simply an extra 8B per duplicate. It also makes the logic simpler and the untrack hook is happy to ignore entries previously removed. Reported-by: Jussi Maki Signed-off-by: John Fastabend --- include/linux/bpf.h | 1 + kernel/bpf/core.c | 6 ++++-- kernel/bpf/verifier.c | 36 +++++++++++++++++++++++++----------- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 02b02cb29ce2..c037c67347c0 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1780,6 +1780,7 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd, return bpf_prog_get_type_dev(ufd, type, false); } +void bpf_free_used_maps(struct bpf_prog_aux *aux); void __bpf_free_used_maps(struct bpf_prog_aux *aux, struct bpf_map **used_maps, u32 len); diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 5e31ee9f7512..ce5bb8932958 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2167,7 +2167,7 @@ void __bpf_free_used_maps(struct bpf_prog_aux *aux, } } -static void bpf_free_used_maps(struct bpf_prog_aux *aux) +void bpf_free_used_maps(struct bpf_prog_aux *aux) { __bpf_free_used_maps(aux, aux->used_maps, aux->used_map_cnt); kfree(aux->used_maps); @@ -2211,8 +2211,10 @@ static void bpf_prog_free_deferred(struct work_struct *work) #endif if (aux->dst_trampoline) bpf_trampoline_put(aux->dst_trampoline); - for (i = 0; i < aux->func_cnt; i++) + for (i = 0; i < aux->func_cnt; i++) { + bpf_free_used_maps(aux->func[i]->aux); bpf_jit_free(aux->func[i]); + } if (aux->func_cnt) { kfree(aux->func); bpf_prog_unlock_free(aux->prog); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 066fac9b5460..31c0f3ad9626 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -12128,14 +12128,32 @@ static int jit_subprogs(struct bpf_verifier_env *env) func[i]->insnsi[insn_idx - subprog_start].imm = ret + 1; } - for (j = 0; j < func[i]->aux->size_poke_tab; j++) { - int ret; + /* overapproximate the number of map slots. Untrack will just skip + * the lookup anyways and we avoid an extra layer of accounting. + */ + if (func[i]->aux->size_poke_tab) { + struct bpf_map **used_maps; - map_ptr = func[i]->aux->poke_tab[j].tail_call.map; - ret = map_ptr->ops->map_poke_track(map_ptr, func[i]->aux); - if (ret < 0) { - verbose(env, "tracking tail call prog failed\n"); + used_maps = kmalloc_array(func[i]->aux->size_poke_tab, + sizeof(struct bpf_map *), + GFP_KERNEL); + if (!used_maps) goto out_free; + + func[i]->aux->used_maps = used_maps; + + for (j = 0; j < func[i]->aux->size_poke_tab; j++) { + int ret; + + map_ptr = func[i]->aux->poke_tab[j].tail_call.map; + ret = map_ptr->ops->map_poke_track(map_ptr, func[i]->aux); + if (ret < 0) { + verbose(env, "tracking tail call prog failed\n"); + goto out_free; + } + bpf_map_inc(map_ptr); + func[i]->aux->used_map_cnt++; + func[i]->aux->used_maps[j] = map_ptr; } } @@ -12259,11 +12277,7 @@ static int jit_subprogs(struct bpf_verifier_env *env) for (i = 0; i < env->subprog_cnt; i++) { if (!func[i]) continue; - - for (j = 0; j < func[i]->aux->size_poke_tab; j++) { - map_ptr = func[i]->aux->poke_tab[j].tail_call.map; - map_ptr->ops->map_poke_untrack(map_ptr, func[i]->aux); - } + bpf_free_used_maps(func[i]->aux); bpf_jit_free(func[i]); } kfree(func); From patchwork Wed Jun 16 22:55:57 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 12326163 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 87770C48BE5 for ; Wed, 16 Jun 2021 22:56:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 60B99613CB for ; Wed, 16 Jun 2021 22:56:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231249AbhFPW6V (ORCPT ); Wed, 16 Jun 2021 18:58:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231419AbhFPW6U (ORCPT ); Wed, 16 Jun 2021 18:58:20 -0400 Received: from mail-io1-xd2b.google.com (mail-io1-xd2b.google.com [IPv6:2607:f8b0:4864:20::d2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 67697C061574 for ; Wed, 16 Jun 2021 15:56:11 -0700 (PDT) Received: by mail-io1-xd2b.google.com with SMTP id 5so997172ioe.1 for ; Wed, 16 Jun 2021 15:56:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=3DLASFQ15Z81rloOq6n84/zQvsm6YmqeDbgPpYmXx0A=; b=p1wQF/DPc15yWjfECOkMCxK5ue6n1s4xTNEUohTbIgU7wHjjfk0rnvsCAOTsNa33tg +0/KFTiwv7HeYU9FGPUyzNgZcDJV8nnp4IIxmv7uloSIbdNlb/GsENI7mA4IUS2JrkvW kiu+5zaTJoFM/qYk2Bwua3yvFYp6zHewknSGo8Adf0kaCxPp+5X9HYUNgNYSlVqYgPKg wOO+lZLOzPExHWxubDDJgp2dmKPIu7iU5gpk5HaVK7SuauTKtuKmNMjCBci/W8kjitns s2u3Y77cC30WBk5n4ZpNOL8YUhlktqBlh85NMeFBOVVccbAtlUd4loSxCspSmXqIK1q/ SaSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=3DLASFQ15Z81rloOq6n84/zQvsm6YmqeDbgPpYmXx0A=; b=F7X/S5oletRuqRXB7HaE3Q4BnpJ6ve9f7euGIiNJtKhSrP1McsQxR/0fRO59W9XsVg qCjsw/ZWIZmzSn1fwmhxlKaXTLm05r9V0kjdMSlpuKVP7r+V7hqM+gMQaxU0enVjfZH+ UbnBYdpg34D8AbutMalln2bsKvHvVF8VZCQZUF4XmGWy2NHlkQ3SUO2Ik2CPq3B037dW Q0RfQ9NwIHoOyi91qLfdN08K2grcNz+Lr4X7KVEuTeEhDXyFCAGqu3aTsXfwj8Nt8ilm 4FIXEVXhdH6YQj0jYBEbqwUJnRWme65FPZO/r0klC+d/WPQ8YqfoO0MUb8+8WmpL8cqd uaFg== X-Gm-Message-State: AOAM531nMq3z+ZZkuhGzz7RAivYC8MOrFlibZK/uD99h5d9/2JJNUAyK XmyBZo4fv9OfryWN/t3UcuM= X-Google-Smtp-Source: ABdhPJyCVBdgBagf/DYUJEryH3KtpkoowCnB39rkVTChGwAaMpZuF5JE40FUYwysgY37xhoLFjbv/A== X-Received: by 2002:a05:6602:2a43:: with SMTP id k3mr1314309iov.47.1623884170683; Wed, 16 Jun 2021 15:56:10 -0700 (PDT) Received: from [127.0.1.1] ([172.243.157.240]) by smtp.gmail.com with ESMTPSA id n2sm1908745iod.54.2021.06.16.15.56.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jun 2021 15:56:10 -0700 (PDT) Subject: [PATCH bpf v2 4/4] bpf: selftest to verify mixing bpf2bpf calls and tailcalls with insn patch From: John Fastabend To: maciej.fijalkowski@intel.com, ast@kernel.org, daniel@iogearbox.net, andriin@fb.com Cc: john.fastabend@gmail.com, netdev@vger.kernel.org, netdev@vger.kernel.org Date: Wed, 16 Jun 2021 15:55:57 -0700 Message-ID: <162388415754.151936.11542697725599301296.stgit@john-XPS-13-9370> In-Reply-To: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> References: <162388400488.151936.1658153981415911010.stgit@john-XPS-13-9370> User-Agent: StGit/0.23-85-g6af9 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net This adds some extra noise to the tailcall_bpf2bpf4 tests that will cause verify to patch insns. This then moves around subprog start/end insn index and poke descriptor insn index to ensure that verify and JIT will continue to track these correctly. If done correctly verifier should pass this program same as before and JIT should emit tail call logic. Signed-off-by: John Fastabend --- tools/testing/selftests/bpf/prog_tests/tailcalls.c | 36 ++++++++++++++------ .../selftests/bpf/progs/tailcall_bpf2bpf4.c | 20 +++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/tailcalls.c b/tools/testing/selftests/bpf/prog_tests/tailcalls.c index ee27d68d2a1c..b5940e6ca67c 100644 --- a/tools/testing/selftests/bpf/prog_tests/tailcalls.c +++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c @@ -715,6 +715,8 @@ static void test_tailcall_bpf2bpf_3(void) bpf_object__close(obj); } +#include "tailcall_bpf2bpf4.skel.h" + /* test_tailcall_bpf2bpf_4 checks that tailcall counter is correctly preserved * across tailcalls combined with bpf2bpf calls. for making sure that tailcall * counter behaves correctly, bpf program will go through following flow: @@ -727,10 +729,15 @@ static void test_tailcall_bpf2bpf_3(void) * the loop begins. At the end of the test make sure that the global counter is * equal to 31, because tailcall counter includes the first two tailcalls * whereas global counter is incremented only on loop presented on flow above. + * + * The noise parameter is used to insert bpf_map_update calls into the logic + * to force verifier to patch instructions. This allows us to ensure jump + * logic remains correct with instruction movement. */ -static void test_tailcall_bpf2bpf_4(void) +static void test_tailcall_bpf2bpf_4(bool noise) { - int err, map_fd, prog_fd, main_fd, data_fd, i, val; + int err, map_fd, prog_fd, main_fd, data_fd, i; + struct tailcall_bpf2bpf4__bss val; struct bpf_map *prog_array, *data_map; struct bpf_program *prog; struct bpf_object *obj; @@ -774,11 +781,6 @@ static void test_tailcall_bpf2bpf_4(void) goto out; } - err = bpf_prog_test_run(main_fd, 1, &pkt_v4, sizeof(pkt_v4), 0, - &duration, &retval, NULL); - CHECK(err || retval != sizeof(pkt_v4) * 3, "tailcall", "err %d errno %d retval %d\n", - err, errno, retval); - data_map = bpf_object__find_map_by_name(obj, "tailcall.bss"); if (CHECK_FAIL(!data_map || !bpf_map__is_internal(data_map))) return; @@ -787,10 +789,22 @@ static void test_tailcall_bpf2bpf_4(void) if (CHECK_FAIL(map_fd < 0)) return; + i = 0; + val.noise = noise; + val.count = 0; + err = bpf_map_update_elem(data_fd, &i, &val, BPF_ANY); + if (CHECK_FAIL(err)) + goto out; + + err = bpf_prog_test_run(main_fd, 1, &pkt_v4, sizeof(pkt_v4), 0, + &duration, &retval, NULL); + CHECK(err || retval != sizeof(pkt_v4) * 3, "tailcall", "err %d errno %d retval %d\n", + err, errno, retval); + i = 0; err = bpf_map_lookup_elem(data_fd, &i, &val); - CHECK(err || val != 31, "tailcall count", "err %d errno %d count %d\n", - err, errno, val); + CHECK(err || val.count != 31, "tailcall count", "err %d errno %d count %d\n", + err, errno, val.count); out: bpf_object__close(obj); @@ -815,5 +829,7 @@ void test_tailcalls(void) if (test__start_subtest("tailcall_bpf2bpf_3")) test_tailcall_bpf2bpf_3(); if (test__start_subtest("tailcall_bpf2bpf_4")) - test_tailcall_bpf2bpf_4(); + test_tailcall_bpf2bpf_4(false); + if (test__start_subtest("tailcall_bpf2bpf_5")) + test_tailcall_bpf2bpf_4(true); } diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c index 9a1b166b7fbe..e89368a50b97 100644 --- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c +++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c @@ -2,6 +2,13 @@ #include #include +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __uint(key_size, sizeof(__u32)); + __uint(value_size, sizeof(__u32)); +} nop_table SEC(".maps"); + struct { __uint(type, BPF_MAP_TYPE_PROG_ARRAY); __uint(max_entries, 3); @@ -9,11 +16,22 @@ struct { __uint(value_size, sizeof(__u32)); } jmp_table SEC(".maps"); -static volatile int count; +int count = 0; +int noise = 0; + +__always_inline int subprog_noise(void) +{ + __u32 key = 0; + + bpf_map_lookup_elem(&nop_table, &key); + return 0; +} __noinline int subprog_tail_2(struct __sk_buff *skb) { + if (noise) + subprog_noise(); bpf_tail_call_static(skb, &jmp_table, 2); return skb->len * 3; }