From patchwork Thu Jun 25 23:12:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 217094 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=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, 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 A1E86C433E0 for ; Thu, 25 Jun 2020 23:13:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 754C020768 for ; Thu, 25 Jun 2020 23:13:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bqreES/u" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407717AbgFYXNN (ORCPT ); Thu, 25 Jun 2020 19:13:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43382 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2407700AbgFYXNN (ORCPT ); Thu, 25 Jun 2020 19:13:13 -0400 Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CC678C08C5C1; Thu, 25 Jun 2020 16:13:12 -0700 (PDT) Received: by mail-il1-x144.google.com with SMTP id z2so6953903ilq.0; Thu, 25 Jun 2020 16:13:12 -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=UlKzt5nRZLioHdCv3xpUOKjebaw4JUUCMMsmEh0wQCk=; b=bqreES/ugvZwco+Gs2GPFWiqG7p+E4m79153g+MXI5LHoHXkHvgWqrxvvI7F/in11p qXCfuIDsH28sigiDn909/SZrllUJC6G82Zlf3pVMPK+ZUHaXewtEV6NwqOcyxES0pJhc MrgU5wZGG9bUDEfngS9Gb2NlvWZSMfs9/IR2olan11Bi8mnC/MaT08oGX3wDAsVM+jQA kv858w+BcVrgj3mq7+HkeHAxv1JMJJsBFf8n4z70c45N3a07q6BjTmP3QdpK89AV+UhD BW2R8XynD0ldErBjPoQ2ltLiGPuqf86j1t15Pvw5Ps/EE+hm5pPrcVd51smS/L3hX41y Zzyw== 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=UlKzt5nRZLioHdCv3xpUOKjebaw4JUUCMMsmEh0wQCk=; b=ouN+u2afd4cs8mZJN398XcBfXdR/nXQIG7JCEt0pT38qMMNsErS+/V1PEQqNNDMXoA mL8Mr48m8QHD/fy9pXte5zR951Ayb+Awp8YduW5DUzAUIuEeph81yNSBa3yGAb8MqhEe 5iSwapnjcKYjIa48eo9TVJ/vWzwXZBFCQHyKwzae8e2TogXMgD5ysfH8dz09uojRKcrz g+beCmhDk3ENkDARoDnjX3Ds1OeRTfZd742RSW/yRLTzRWQ/KIJHm7jChKmzxSiOl97U 5tY8RArO+JgSMoclDc+X7xq9hcl+OBLWloGaMtBdiHxQf34iofxZl6O4YlB8KRUkIqGd JKoA== X-Gm-Message-State: AOAM533XEft8FnK3pbEbZpgz9LjGRNACz1dcBqHV8JX5GkFfRc0+jWmt xHHA8mBHdG2qSQPutF+Ybms= X-Google-Smtp-Source: ABdhPJzKjvYzwAMjNA+DWze8GEhPLX6UGfsD3gB/l9B0B8bRvMam2zzeL8AqiNlg5J6BGwE4JSKoHg== X-Received: by 2002:a92:2a06:: with SMTP id r6mr304935ile.121.1593126792184; Thu, 25 Jun 2020 16:13:12 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id b8sm14386638ior.35.2020.06.25.16.13.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jun 2020 16:13:11 -0700 (PDT) Subject: [bpf PATCH v2 1/3] bpf, sockmap: RCU splat with redirect and strparser error or TLS From: John Fastabend To: kafai@fb.com, jakub@cloudflare.com, daniel@iogearbox.net, ast@kernel.org Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, john.fastabend@gmail.com Date: Thu, 25 Jun 2020 16:12:59 -0700 Message-ID: <159312677907.18340.11064813152758406626.stgit@john-XPS-13-9370> In-Reply-To: <159312606846.18340.6821004346409614051.stgit@john-XPS-13-9370> References: <159312606846.18340.6821004346409614051.stgit@john-XPS-13-9370> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org There are two paths to generate the below RCU splat the first and most obvious is the result of the BPF verdict program issuing a redirect on a TLS socket (This is the splat shown below). Unlike the non-TLS case the caller of the *strp_read() hooks does not wrap the call in a rcu_read_lock/unlock. Then if the BPF program issues a redirect action we hit the RCU splat. However, in the non-TLS socket case the splat appears to be relatively rare, because the skmsg caller into the strp_data_ready() is wrapped in a rcu_read_lock/unlock. Shown here, static void sk_psock_strp_data_ready(struct sock *sk) { struct sk_psock *psock; rcu_read_lock(); psock = sk_psock(sk); if (likely(psock)) { if (tls_sw_has_ctx_rx(sk)) { psock->parser.saved_data_ready(sk); } else { write_lock_bh(&sk->sk_callback_lock); strp_data_ready(&psock->parser.strp); write_unlock_bh(&sk->sk_callback_lock); } } rcu_read_unlock(); } If the above was the only way to run the verdict program we would be safe. But, there is a case where the strparser may throw an ENOMEM error while parsing the skb. This is a result of a failed skb_clone, or alloc_skb_for_msg while building a new merged skb when the msg length needed spans multiple skbs. This will in turn put the skb on the strp_wrk workqueue in the strparser code. The skb will later be dequeued and verdict programs run, but now from a different context without the rcu_read_lock()/unlock() critical section in sk_psock_strp_data_ready() shown above. In practice I have not seen this yet, because as far as I know most users of the verdict programs are also only working on single skbs. In this case no merge happens which could trigger the above ENOMEM errors. In addition the system would need to be under memory pressure. For example, we can't hit the above case in selftests because we missed having tests to merge skbs. (Added in later patch) To fix the below splat extend the rcu_read_lock/unnlock block to include the call to sk_psock_tls_verdict_apply(). This will fix both TLS redirect case and non-TLS redirect+error case. Also remove psock from the sk_psock_tls_verdict_apply() function signature its not used there. [ 1095.937597] WARNING: suspicious RCU usage [ 1095.940964] 5.7.0-rc7-02911-g463bac5f1ca79 #1 Tainted: G W [ 1095.944363] ----------------------------- [ 1095.947384] include/linux/skmsg.h:284 suspicious rcu_dereference_check() usage! [ 1095.950866] [ 1095.950866] other info that might help us debug this: [ 1095.950866] [ 1095.957146] [ 1095.957146] rcu_scheduler_active = 2, debug_locks = 1 [ 1095.961482] 1 lock held by test_sockmap/15970: [ 1095.964501] #0: ffff9ea6b25de660 (sk_lock-AF_INET){+.+.}-{0:0}, at: tls_sw_recvmsg+0x13a/0x840 [tls] [ 1095.968568] [ 1095.968568] stack backtrace: [ 1095.975001] CPU: 1 PID: 15970 Comm: test_sockmap Tainted: G W 5.7.0-rc7-02911-g463bac5f1ca79 #1 [ 1095.977883] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 1095.980519] Call Trace: [ 1095.982191] dump_stack+0x8f/0xd0 [ 1095.984040] sk_psock_skb_redirect+0xa6/0xf0 [ 1095.986073] sk_psock_tls_strp_read+0x1d8/0x250 [ 1095.988095] tls_sw_recvmsg+0x714/0x840 [tls] v2: Improve commit message to identify non-TLS redirect plus error case condition as well as more common TLS case. In the process I decided doing the rcu_read_unlock followed by the lock/unlock inside branches was unnecessarily complex. We can just extend the current rcu block and get the same effeective without the shuffling and branching. Thanks Martin! Fixes: e91de6afa81c1 ("bpf: Fix running sk_skb program types with ktls") Reported-by: Jakub Sitnicki Reported-by: kernel test robot Signed-off-by: John Fastabend --- net/core/skmsg.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/net/core/skmsg.c b/net/core/skmsg.c index 351afbf6bfba..c41ab6906b21 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -683,7 +683,7 @@ static struct sk_psock *sk_psock_from_strp(struct strparser *strp) return container_of(parser, struct sk_psock, parser); } -static void sk_psock_skb_redirect(struct sk_psock *psock, struct sk_buff *skb) +static void sk_psock_skb_redirect(struct sk_buff *skb) { struct sk_psock *psock_other; struct sock *sk_other; @@ -715,12 +715,11 @@ static void sk_psock_skb_redirect(struct sk_psock *psock, struct sk_buff *skb) } } -static void sk_psock_tls_verdict_apply(struct sk_psock *psock, - struct sk_buff *skb, int verdict) +static void sk_psock_tls_verdict_apply(struct sk_buff *skb, int verdict) { switch (verdict) { case __SK_REDIRECT: - sk_psock_skb_redirect(psock, skb); + sk_psock_skb_redirect(skb); break; case __SK_PASS: case __SK_DROP: @@ -741,8 +740,8 @@ int sk_psock_tls_strp_read(struct sk_psock *psock, struct sk_buff *skb) ret = sk_psock_bpf_run(psock, prog, skb); ret = sk_psock_map_verd(ret, tcp_skb_bpf_redirect_fetch(skb)); } + sk_psock_tls_verdict_apply(skb, ret); rcu_read_unlock(); - sk_psock_tls_verdict_apply(psock, skb, ret); return ret; } EXPORT_SYMBOL_GPL(sk_psock_tls_strp_read); @@ -770,7 +769,7 @@ static void sk_psock_verdict_apply(struct sk_psock *psock, } goto out_free; case __SK_REDIRECT: - sk_psock_skb_redirect(psock, skb); + sk_psock_skb_redirect(skb); break; case __SK_DROP: /* fall-through */ @@ -794,8 +793,8 @@ static void sk_psock_strp_read(struct strparser *strp, struct sk_buff *skb) ret = sk_psock_bpf_run(psock, prog, skb); ret = sk_psock_map_verd(ret, tcp_skb_bpf_redirect_fetch(skb)); } - rcu_read_unlock(); sk_psock_verdict_apply(psock, skb, ret); + rcu_read_unlock(); } static int sk_psock_strp_read_done(struct strparser *strp, int err) From patchwork Thu Jun 25 23:13:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 217093 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=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS autolearn=unavailable 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 74A89C433DF for ; Thu, 25 Jun 2020 23:13:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5436C20768 for ; Thu, 25 Jun 2020 23:13:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="e/5wXZyN" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407738AbgFYXNw (ORCPT ); Thu, 25 Jun 2020 19:13:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406631AbgFYXNw (ORCPT ); Thu, 25 Jun 2020 19:13:52 -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 24BF4C08C5C1; Thu, 25 Jun 2020 16:13:52 -0700 (PDT) Received: by mail-io1-xd2f.google.com with SMTP id e64so2985922iof.12; Thu, 25 Jun 2020 16:13: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=2dDp4wdTOXkTlMfK+S1sLZUdmr8B+CfJrwoh3NxWVt8=; b=e/5wXZyNhEhlZ5rpDWJ2/3vVtIupUKHBD/sDdHvqFyE+Bt2HefyUme/ogAM/y9cXYC +8xGgC44veimAb0dTz0SNFhk9Dr6GJzY087XRqtPvmqIOJcs+ecRnjUQQMFjvpDk6JgD w7BLdP8rQoxlFSVSR5vIyZvpi3tMPlovb3cVQj3/+NqDMWaLI/OSQe2cuay7Az7XoSyw LZ1ED3UaSiWAG8pd7LxwG1NET81umigHarqt0BdYkJXwurkZ6RtpKhxza2APssgoj18Y asPGcJ1aPKRb/b4WLXz6tSQdWUMjgO46v+CBT31njbD+coCMkh9aMHgWs7CGybl4lEbo S9LQ== 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=2dDp4wdTOXkTlMfK+S1sLZUdmr8B+CfJrwoh3NxWVt8=; b=bhME/MMILqjvL0zkive0yvH04EazZt6GuvvwVR4T7/qvy+71JTS8b7PRUQy0yDeQCU n4VGn6K/IQv59g0G8a7UWNeZjRiJ7fK2ehepQ6Oo5Jmttz5oQqY297XqDW5Io0/xyR+Z 0xdzOC+TBkm1PZOcWX9UPgDatUixR4ctE4bRScyujgylaYQxgQ6frupmRIlvaPiWAg5O TT83C4TauZmo0mLJyHyzbGpuB8vG1HxvLQMPJ2N6x4UQepOJiAcBRbCm6Kzc5ysb+ic8 sgAZed4copV6/Y1vEYAYEaryIMfc1YbXjUNaBiniDFabBf1ewTThVsi5TUzAg3acJyg/ Wq1Q== X-Gm-Message-State: AOAM533TC5Hz5oEysMTfZnpnHp0l3Z2p2M3M8J7/VFVvxSiqFjMqnY6u 8NUVC8cnqqqJsBig6RwnOBE= X-Google-Smtp-Source: ABdhPJyeAHc7ZaCtUtouKsmYR+vIfTgTh+QRzXwQoaMUp9ppuRgtnlOCho+dG7XIv7hvcdkuAczElA== X-Received: by 2002:a05:6602:2dd4:: with SMTP id l20mr502009iow.13.1593126831515; Thu, 25 Jun 2020 16:13:51 -0700 (PDT) Received: from [127.0.1.1] ([184.63.162.180]) by smtp.gmail.com with ESMTPSA id e12sm13744538ili.68.2020.06.25.16.13.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jun 2020 16:13:50 -0700 (PDT) Subject: [bpf PATCH v2 3/3] bpf, sockmap: Add ingres skb tests that utilize merge skbs From: John Fastabend To: kafai@fb.com, jakub@cloudflare.com, daniel@iogearbox.net, ast@kernel.org Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, john.fastabend@gmail.com Date: Thu, 25 Jun 2020 16:13:38 -0700 Message-ID: <159312681884.18340.4922800172600252370.stgit@john-XPS-13-9370> In-Reply-To: <159312606846.18340.6821004346409614051.stgit@john-XPS-13-9370> References: <159312606846.18340.6821004346409614051.stgit@john-XPS-13-9370> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Add a test to check strparser merging skbs is working. Signed-off-by: John Fastabend --- .../selftests/bpf/progs/test_sockmap_kern.h | 8 +++++++- tools/testing/selftests/bpf/test_sockmap.c | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h index 057036ca1111..3dca4c2e2418 100644 --- a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h +++ b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h @@ -79,7 +79,7 @@ struct { struct { __uint(type, BPF_MAP_TYPE_ARRAY); - __uint(max_entries, 2); + __uint(max_entries, 3); __type(key, int); __type(value, int); } sock_skb_opts SEC(".maps"); @@ -94,6 +94,12 @@ struct { SEC("sk_skb1") int bpf_prog1(struct __sk_buff *skb) { + int *f, two = 2; + + f = bpf_map_lookup_elem(&sock_skb_opts, &two); + if (f && *f) { + return *f; + } return skb->len; } diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c index 37695fc8096a..78789b27e573 100644 --- a/tools/testing/selftests/bpf/test_sockmap.c +++ b/tools/testing/selftests/bpf/test_sockmap.c @@ -85,6 +85,7 @@ int txmsg_ktls_skb_drop; int txmsg_ktls_skb_redir; int ktls; int peek_flag; +int skb_use_parser; static const struct option long_options[] = { {"help", no_argument, NULL, 'h' }, @@ -174,6 +175,7 @@ static void test_reset(void) txmsg_apply = txmsg_cork = 0; txmsg_ingress = txmsg_redir_skb = 0; txmsg_ktls_skb = txmsg_ktls_skb_drop = txmsg_ktls_skb_redir = 0; + skb_use_parser = 0; } static int test_start_subtest(const struct _test *t, struct sockmap_options *o) @@ -1211,6 +1213,11 @@ static int run_options(struct sockmap_options *options, int cg_fd, int test) } } + if (skb_use_parser) { + i = 2; + err = bpf_map_update_elem(map_fd[7], &i, &skb_use_parser, BPF_ANY); + } + if (txmsg_drop) options->drop_expected = true; @@ -1650,6 +1657,16 @@ static void test_txmsg_cork(int cgrp, struct sockmap_options *opt) test_send(opt, cgrp); } +static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt) +{ + txmsg_pass = 1; + skb_use_parser = 512; + opt->iov_length = 256; + opt->iov_count = 1; + opt->rate = 2; + test_exec(cgrp, opt); +} + char *map_names[] = { "sock_map", "sock_map_txmsg", @@ -1748,6 +1765,7 @@ struct _test test[] = { {"txmsg test pull-data", test_txmsg_pull}, {"txmsg test pop-data", test_txmsg_pop}, {"txmsg test push/pop data", test_txmsg_push_pop}, + {"txmsg text ingress parser", test_txmsg_ingress_parser}, }; static int check_whitelist(struct _test *t, struct sockmap_options *opt)