Message ID | 1701252962-63418-1-git-send-email-alibuda@linux.alibaba.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a5a7:0:b0:403:3b70:6f57 with SMTP id d7csp239083vqn; Wed, 29 Nov 2023 02:16:35 -0800 (PST) X-Google-Smtp-Source: AGHT+IGi4xVEvV0CV656fNNQAy91+CtkXtKKfaCCOIJdv3oK8RnBCWS0I+1ZB9796qBAQAaWgXOs X-Received: by 2002:a05:6870:d248:b0:1fa:306f:df07 with SMTP id h8-20020a056870d24800b001fa306fdf07mr12672171oac.19.1701252995739; Wed, 29 Nov 2023 02:16:35 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701252995; cv=none; d=google.com; s=arc-20160816; b=S7gPyz7WADQOckz+4VCKv5h/gEnYdxrayK0ANurBJa1bVtsRqSK0H1aDmFbt5PDBI4 sO++V+6SyE/i2AkEBKpslcPP8fLQrik8XF/8huv0EdUu9wh0zbo9Guc09e53BHsa2Q70 XMIh8C9dP/9AC8n0WaMD9ZNVVVfoZNnOm+jDCyDxxOj4++LFz3jCEFRSrgxMi9CE83/i sMRdJqE0E1Edw0JJLYO6iRvjiTrlzPtrC68Tky37AvDwNylJM1YnRp/pz6yA0IUEM1ow g/DbwMFyR4gxc7l5kDshbb16rskM9UusFaMX+XVyPjFFtUommjvd/Qoo1ZSFkg61gHu9 BxwA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from; bh=IaiguJmGzt8Qs2jOfeprncfmigP+u7Nx5h2rjrxtxNw=; fh=0j9hEtKEGSulx/lgeQcpDl+4ujiVFr5DCPGOZLIRfAc=; b=suSjX8krrZ7Rzzc6svT3bUh5bR6mR1rQfGoDyWXx47ittljwDCy56MKVXPDJY8e/kh K63lE6unQI7SfYXWj9HA8EfhvZS7YyWoYOo6eGJu74lQscx7mS9xLf6srnMyeJoW0hE3 NkZVpa+Bm2o8fzy2K87wfBLhtchL8V4jbqBhcJetvJsHgWH+G4BjkMzvJ3qNY6/dqZPE QH6++gcevuQIEWgcLMMCvAK3HPBP7kQoz0DsIeTM706QFuIfUY9FJB8Q/8+KxLpPiFjO qWC4qvQ47k8T9AOOCWLbb9cDur4azOMqdUyCTyxS0KYiuKiCrXQV02LvHPnzfFmIVU+N X6AQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id by37-20020a056a0205a500b005b942de1e92si14931819pgb.443.2023.11.29.02.16.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Nov 2023 02:16:35 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id EDFED806AA2B; Wed, 29 Nov 2023 02:16:32 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229848AbjK2KQG (ORCPT <rfc822;toshivichauhan@gmail.com> + 99 others); Wed, 29 Nov 2023 05:16:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48798 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229509AbjK2KQE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 29 Nov 2023 05:16:04 -0500 Received: from out30-130.freemail.mail.aliyun.com (out30-130.freemail.mail.aliyun.com [115.124.30.130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6696FC4; Wed, 29 Nov 2023 02:16:10 -0800 (PST) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R151e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046050;MF=alibuda@linux.alibaba.com;NM=1;PH=DS;RN=13;SR=0;TI=SMTPD_---0VxNr3Ss_1701252962; Received: from j66a10360.sqa.eu95.tbsite.net(mailfrom:alibuda@linux.alibaba.com fp:SMTPD_---0VxNr3Ss_1701252962) by smtp.aliyun-inc.com; Wed, 29 Nov 2023 18:16:08 +0800 From: "D. Wythe" <alibuda@linux.alibaba.com> To: pablo@netfilter.org, kadlec@netfilter.org, fw@strlen.de Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, coreteam@netfilter.org, netfilter-devel@vger.kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, ast@kernel.org Subject: [PATCH net] net/netfilter: bpf: avoid leakage of skb Date: Wed, 29 Nov 2023 18:16:02 +0800 Message-Id: <1701252962-63418-1-git-send-email-alibuda@linux.alibaba.com> X-Mailer: git-send-email 1.8.3.1 X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 29 Nov 2023 02:16:33 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783893061364531060 X-GMAIL-MSGID: 1783893061364531060 |
Series |
[net] net/netfilter: bpf: avoid leakage of skb
|
|
Commit Message
D. Wythe
Nov. 29, 2023, 10:16 a.m. UTC
From: "D. Wythe" <alibuda@linux.alibaba.com> A malicious eBPF program can interrupt the subsequent processing of a skb by returning an exceptional retval, and no one will be responsible for releasing the very skb. Moreover, normal programs can also have the demand to return NF_STOLEN, usually, the hook needs to take responsibility for releasing this skb itself, but currently, there is no such helper function to achieve that. Ignoring NF_STOLEN will also lead to skb leakage. Fixes: fd9c663b9ad6 ("bpf: minimal support for programs hooked into netfilter framework") Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> --- net/netfilter/nf_bpf_link.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
Comments
D. Wythe <alibuda@linux.alibaba.com> wrote: > From: "D. Wythe" <alibuda@linux.alibaba.com> > > A malicious eBPF program can interrupt the subsequent processing of > a skb by returning an exceptional retval, and no one will be responsible > for releasing the very skb. How? The bpf verifier is supposed to reject nf bpf programs that return a value other than accept or drop. If this is a real bug, please also figure out why 006c0e44ed92 ("selftests/bpf: add missing netfilter return value and ctx access tests") failed to catch it. > Moreover, normal programs can also have the demand to return NF_STOLEN, No, this should be disallowed already. > net/netfilter/nf_bpf_link.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c > index e502ec0..03c47d6 100644 > --- a/net/netfilter/nf_bpf_link.c > +++ b/net/netfilter/nf_bpf_link.c > @@ -12,12 +12,29 @@ static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, > const struct nf_hook_state *s) > { > const struct bpf_prog *prog = bpf_prog; > + unsigned int verdict; > struct bpf_nf_ctx ctx = { > .state = s, > .skb = skb, > }; > > - return bpf_prog_run(prog, &ctx); > + verdict = bpf_prog_run(prog, &ctx); > + switch (verdict) { > + case NF_STOLEN: > + consume_skb(skb); > + fallthrough; This can't be right. STOLEN really means STOLEN (free'd, redirected, etc, "skb" MUST be "leaked". Which is also why the bpf program is not allowed to return it.
On 11/29/23 9:18 PM, Florian Westphal wrote: > D. Wythe <alibuda@linux.alibaba.com> wrote: >> From: "D. Wythe" <alibuda@linux.alibaba.com> >> >> A malicious eBPF program can interrupt the subsequent processing of >> a skb by returning an exceptional retval, and no one will be responsible >> for releasing the very skb. > How? The bpf verifier is supposed to reject nf bpf programs that > return a value other than accept or drop. > > If this is a real bug, please also figure out why > 006c0e44ed92 ("selftests/bpf: add missing netfilter return value and ctx access tests") > failed to catch it. Hi Florian, You are right, i make a mistake.. , it's not a bug.. And my origin intention was to allow ebpf progs to return NF_STOLEN, we are trying to modify some netfilter modules via ebpf, and some scenarios require the use of NF_STOLEN, but from your description, it seems that at least currently, you do not want to return NF_STOLEN, until there is a helper for sonsume_skb(), right ? Again, very sorry to bother you. Best wishes, D. Wythe. >> Moreover, normal programs can also have the demand to return NF_STOLEN, > No, this should be disallowed already. > >> net/netfilter/nf_bpf_link.c | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c >> index e502ec0..03c47d6 100644 >> --- a/net/netfilter/nf_bpf_link.c >> +++ b/net/netfilter/nf_bpf_link.c >> @@ -12,12 +12,29 @@ static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, >> const struct nf_hook_state *s) >> { >> const struct bpf_prog *prog = bpf_prog; >> + unsigned int verdict; >> struct bpf_nf_ctx ctx = { >> .state = s, >> .skb = skb, >> }; >> >> - return bpf_prog_run(prog, &ctx); >> + verdict = bpf_prog_run(prog, &ctx); >> + switch (verdict) { >> + case NF_STOLEN: >> + consume_skb(skb); >> + fallthrough; > This can't be right. STOLEN really means STOLEN (free'd, > redirected, etc, "skb" MUST be "leaked". > > Which is also why the bpf program is not allowed to return it.
D. Wythe <alibuda@linux.alibaba.com> wrote: > And my origin intention was to allow ebpf progs to return NF_STOLEN, we are > trying to modify some netfilter modules via ebpf, > and some scenarios require the use of NF_STOLEN, but from your description, NF_STOLEN can only be supported via a trusted helper, as least as far as I understand. Otherwise verifier would have to guarantee that any branch that returns NF_STOLEN has released the skb, or passed it to a function that will release the skb in the near future.
On 11/29/23 10:47 PM, Florian Westphal wrote: > D. Wythe <alibuda@linux.alibaba.com> wrote: >> And my origin intention was to allow ebpf progs to return NF_STOLEN, we are >> trying to modify some netfilter modules via ebpf, >> and some scenarios require the use of NF_STOLEN, but from your description, > NF_STOLEN can only be supported via a trusted helper, as least as far as > I understand. > > Otherwise verifier would have to guarantee that any branch that returns > NF_STOLEN has released the skb, or passed it to a function that will > release the skb in the near future. Thank you very much for your help. I now understand the difficulty here. The verifier cannot determine whether the consume_skb() was executed or not, when the return value goes to NF_STOLEN. We may use NF_DROP at first, it won't be make much difference for us now. Also, do you have any plans to support this helper? Best wishes, D. Wythe
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c index e502ec0..03c47d6 100644 --- a/net/netfilter/nf_bpf_link.c +++ b/net/netfilter/nf_bpf_link.c @@ -12,12 +12,29 @@ static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, const struct nf_hook_state *s) { const struct bpf_prog *prog = bpf_prog; + unsigned int verdict; struct bpf_nf_ctx ctx = { .state = s, .skb = skb, }; - return bpf_prog_run(prog, &ctx); + verdict = bpf_prog_run(prog, &ctx); + switch (verdict) { + case NF_STOLEN: + consume_skb(skb); + fallthrough; + case NF_ACCEPT: + case NF_DROP: + case NF_QUEUE: + /* restrict the retval of the ebpf programs */ + break; + default: + /* force it to be dropped */ + verdict = NF_DROP_ERR(-EINVAL); + break; + } + + return verdict; } struct bpf_nf_link {