From patchwork Tue Apr 5 17:03:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tadeusz Struk X-Patchwork-Id: 556497 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 82B50C41535 for ; Tue, 5 Apr 2022 21:24:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347294AbiDEVW4 (ORCPT ); Tue, 5 Apr 2022 17:22:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1458052AbiDERGa (ORCPT ); Tue, 5 Apr 2022 13:06:30 -0400 Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 063A6AC06E for ; Tue, 5 Apr 2022 10:04:32 -0700 (PDT) Received: by mail-pf1-x42a.google.com with SMTP id h19so47021pfv.1 for ; Tue, 05 Apr 2022 10:04:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=pROGuwStiro9rcSYW3YeAtKJaGC9ZWqC+E1qhRzShxk=; b=tA6Lp2KTlzqbzCItB0ys8viPQghH3YJ6+SdbGwcrPNlITBfn8nt3mBaneYmi6Ifz19 CJiVBsCpL+c6HqbbfYgHzDZsuF/gkKXK2lc4bTgNxgAK0/S4mz/rsh1hqwAlMUKEjKul w4GTz21a+EhIjdikEvLqtLFgIfLRaNS4O1VLUqQ4S1I5lgeSNNvm0TyL80b59lj1OXUa hXj0ircE3zWV8P0ASeIP76xonjuC1AoHwd7xnKPxA8+7Ewp9e+chhvgblMU270xglxj+ rlavD5UNtimCx/YI+j+fTfO8nmEkwYIbCLnqEcie9L/jpvqMoWWcBJR7XFjpUQvB+ZAa 7E8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=pROGuwStiro9rcSYW3YeAtKJaGC9ZWqC+E1qhRzShxk=; b=uym2I+AqN66E5kq+oFIfp26BXnj59K73aBXgKEqKGugSD2vUIbopCwLa2xGxdEFnkP aHUZBNKXwT+XptsARAA3uz7DoRo0MH/SHgseLetLULkk80O0Ln1MKn6jAjM9+fbawMyL MYHmA17oBLumjNMJUN2+pb3//ta/31ESTKuLe8SfxvDVLL1trz72Z/723y4k2FpSt3ET 8CaDqbj5TdUVh36frjCVQo7LL1Fl2E4PtC9YwXXp7uIH2tau3fLdKzmQbpmUZaK9oacZ VD8WvqtXTLJ/wg7TpDR6mvDMITkPu1d5Jry946a923V9bPZAuVEq6Cb+kXdBdNjHOZlo qGwg== X-Gm-Message-State: AOAM533HQkFtvn5tqfOvRxfO22W6C0+Wo/uOF0bxc9igUpni8S+4Btys ShteM53kWnKF16M5+8gOEd5A3g== X-Google-Smtp-Source: ABdhPJwD7B9L7RiPiFfKaGMQkGtYLA2hahKEheRhiKEh42v4f/MZK/KIfpScH9FOO2wpWsopqWEb2w== X-Received: by 2002:a65:4647:0:b0:399:11b1:810b with SMTP id k7-20020a654647000000b0039911b1810bmr3638651pgr.449.1649178271389; Tue, 05 Apr 2022 10:04:31 -0700 (PDT) Received: from localhost.localdomain ([50.39.160.154]) by smtp.gmail.com with ESMTPSA id m7-20020a625807000000b004fe0a89f24fsm7796142pfb.112.2022.04.05.10.04.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Apr 2022 10:04:30 -0700 (PDT) From: Tadeusz Struk To: bpf@vger.kernel.org Cc: Tadeusz Struk , "Alexei Starovoitov" , "Daniel Borkmann" , "Andrii Nakryiko" , "Martin KaFai Lau" , "Song Liu" , "Yonghong Song" , "John Fastabend" , "KP Singh" , netdev@vger.kernel.org, stable@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+f264bffdfbd5614f3bb2@syzkaller.appspotmail.com Subject: [PATCH] bpf: Fix KASAN use-after-free Read in compute_effective_progs Date: Tue, 5 Apr 2022 10:03:56 -0700 Message-Id: <20220405170356.43128-1-tadeusz.struk@linaro.org> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Syzbot found a Use After Free bug in compute_effective_progs(). The reproducer creates a number of BPF links, and causes a fault injected alloc to fail, while calling bpf_link_detach on them. Link detach triggers the link to be freed by bpf_link_free(), which calls __cgroup_bpf_detach() and update_effective_progs(). If the memory allocation in this function fails, the function restores the pointer to the bpf_cgroup_link on the cgroup list, but the memory gets freed just after it returns. After this, every subsequent call to update_effective_progs() causes this already deallocated pointer to be dereferenced in prog_list_length(), and triggers KASAN UAF error. To fix this don't preserve the pointer to the link on the cgroup list in __cgroup_bpf_detach(), but proceed with the cleanup and retry calling update_effective_progs() again afterwards. Cc: "Alexei Starovoitov" Cc: "Daniel Borkmann" Cc: "Andrii Nakryiko" Cc: "Martin KaFai Lau" Cc: "Song Liu" Cc: "Yonghong Song" Cc: "John Fastabend" Cc: "KP Singh" Cc: Cc: Cc: Cc: Link: https://syzkaller.appspot.com/bug?id=8ebf179a95c2a2670f7cf1ba62429ec044369db4 Fixes: af6eea57437a ("bpf: Implement bpf_link-based cgroup BPF program attachment") Reported-by: Signed-off-by: Tadeusz Struk --- kernel/bpf/cgroup.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 128028efda64..b6307337a3c7 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -723,10 +723,11 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog, pl->link = NULL; err = update_effective_progs(cgrp, atype); - if (err) - goto cleanup; - - /* now can actually delete it from this cgroup list */ + /* + * Proceed regardless of error. The link and/or prog will be freed + * just after this function returns so just delete it from this + * cgroup list and retry calling update_effective_progs again later. + */ list_del(&pl->node); kfree(pl); if (list_empty(progs)) @@ -735,12 +736,11 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog, if (old_prog) bpf_prog_put(old_prog); static_branch_dec(&cgroup_bpf_enabled_key[atype]); - return 0; -cleanup: - /* restore back prog or link */ - pl->prog = old_prog; - pl->link = link; + /* In case of error call update_effective_progs again */ + if (err) + err = update_effective_progs(cgrp, atype); + return err; } @@ -881,6 +881,7 @@ static void bpf_cgroup_link_release(struct bpf_link *link) struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link); struct cgroup *cg; + int err; /* link might have been auto-detached by dying cgroup already, * in that case our work is done here @@ -896,8 +897,10 @@ static void bpf_cgroup_link_release(struct bpf_link *link) return; } - WARN_ON(__cgroup_bpf_detach(cg_link->cgroup, NULL, cg_link, - cg_link->type)); + err = __cgroup_bpf_detach(cg_link->cgroup, NULL, cg_link, + cg_link->type); + if (err) + pr_warn("cgroup_bpf_detach() failed, err %d\n", err); cg = cg_link->cgroup; cg_link->cgroup = NULL;