From patchwork Tue Oct 26 21:41:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 12585719 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 672A7C433F5 for ; Tue, 26 Oct 2021 21:41:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 433B161002 for ; Tue, 26 Oct 2021 21:41:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237228AbhJZVoH (ORCPT ); Tue, 26 Oct 2021 17:44:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42538 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231207AbhJZVoC (ORCPT ); Tue, 26 Oct 2021 17:44:02 -0400 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5EC7FC061570; Tue, 26 Oct 2021 14:41:38 -0700 (PDT) Received: by mail-pf1-x42b.google.com with SMTP id o133so717393pfg.7; Tue, 26 Oct 2021 14:41:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=/GhbiSeBHjPCFGsQePRlQIpTU8EMSfsPXRuz5Bl60t0=; b=dpEtGRKWujjBgcwz6UG0AN9j9qBmBR1cR4fUZvHPFGf1C/pSwwN35j8I1TQA14PUQR a67Sm92v+VSAUPymBFp9Y1CIiEBBAmbhG8wn6Hw3G/MlSwwcQpHNIYiARnaB2Ccby1YE 7Fk2cktGRjxi1qgR87CaD2fO9Bw/tM0My6fdnc6i6ckD7V0xfiHHyQA2/Ej7/zR6CG1U IKt/zR8Vn8lEv1mwTu3WITUZwWyVEksf4E93YkCQIMXQq95So8P3MP91aMZrDq51SJBF qWPM1fs1jTusfRaEhxfGY6Kqd5e82CetC+2f/ez4Ykd7Mp7mHiYKGztGMZdF92WSVkcD SvOA== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=/GhbiSeBHjPCFGsQePRlQIpTU8EMSfsPXRuz5Bl60t0=; b=TlehngnIAeyUEHl+c8iikO8AEUGk2IO5O8FJni6NC2shKWxmQaeeQroLBHLMT5v94E DgH5e8QEeGOpYZH2CbqpYJh+4pcLFpU82u7igPBwoUK49IcWwj9Z64/SyKVAF252AnfC 1pr2KUOJg06Zut/vHMLfi8yObtcvHzRjHjN91vtVAf+TUzcewP1K/DOj/HDqH57K4RNp g4pi6959d9p/fahvQatM2Bv9vi+l9a5al2MT1BuZc9caJTuci/CEI4d+MGJbaDdM6YUw xOk16xGRX2fXcgd/D1tSTS/ZH3fGg/VeE7YjoPVIRO0L83pcLU22OWI6MmxXO9KQK0XZ 1b4w== X-Gm-Message-State: AOAM531L0qKs43o/FhMbGIZwNCMe6/JzTP7ZNa4qmX0Prq8rojz3pfOn xDofwKZ7MUElfh3nTFMXOW0= X-Google-Smtp-Source: ABdhPJxZvpMKjZYP1rv8RNl+rohGTtgpYIbd+Bko1NHDSNSAoEcVJ1fi9r1daRiVGA1YKDhOed3eaA== X-Received: by 2002:a63:6b84:: with SMTP id g126mr21168717pgc.110.1635284497616; Tue, 26 Oct 2021 14:41:37 -0700 (PDT) Received: from edumazet1.svl.corp.google.com ([2620:15c:2c4:201:207c:7102:7bd7:80eb]) by smtp.gmail.com with ESMTPSA id g22sm6495400pfj.181.2021.10.26.14.41.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Oct 2021 14:41:37 -0700 (PDT) From: Eric Dumazet To: Alexei Starovoitov , Daniel Borkmann Cc: "David S . Miller" , netdev , Eric Dumazet , Eric Dumazet , bpf , Andrii Nakryiko , syzbot Subject: [PATCH V2 bpf-next 1/3] bpf: avoid races in __bpf_prog_run() for 32bit arches Date: Tue, 26 Oct 2021 14:41:31 -0700 Message-Id: <20211026214133.3114279-2-eric.dumazet@gmail.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog In-Reply-To: <20211026214133.3114279-1-eric.dumazet@gmail.com> References: <20211026214133.3114279-1-eric.dumazet@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net From: Eric Dumazet __bpf_prog_run() can run from non IRQ contexts, meaning it could be re entered if interrupted. This calls for the irq safe variant of u64_stats_update_{begin|end}, or risk a deadlock. This patch is a nop on 64bit arches, fortunately. syzbot report: WARNING: inconsistent lock state 5.12.0-rc3-syzkaller #0 Not tainted -------------------------------- inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage. udevd/4013 [HC0[0]:SC0[0]:HE1:SE1] takes: ff7c9dec (&(&pstats->syncp)->seq){+.?.}-{0:0}, at: sk_filter include/linux/filter.h:867 [inline] ff7c9dec (&(&pstats->syncp)->seq){+.?.}-{0:0}, at: do_one_broadcast net/netlink/af_netlink.c:1468 [inline] ff7c9dec (&(&pstats->syncp)->seq){+.?.}-{0:0}, at: netlink_broadcast_filtered+0x27c/0x4fc net/netlink/af_netlink.c:1520 {IN-SOFTIRQ-W} state was registered at: lock_acquire.part.0+0xf0/0x41c kernel/locking/lockdep.c:5510 lock_acquire+0x6c/0x74 kernel/locking/lockdep.c:5483 do_write_seqcount_begin_nested include/linux/seqlock.h:520 [inline] do_write_seqcount_begin include/linux/seqlock.h:545 [inline] u64_stats_update_begin include/linux/u64_stats_sync.h:129 [inline] bpf_prog_run_pin_on_cpu include/linux/filter.h:624 [inline] bpf_prog_run_clear_cb+0x1bc/0x270 include/linux/filter.h:755 run_filter+0xa0/0x17c net/packet/af_packet.c:2031 packet_rcv+0xc0/0x3e0 net/packet/af_packet.c:2104 dev_queue_xmit_nit+0x2bc/0x39c net/core/dev.c:2387 xmit_one net/core/dev.c:3588 [inline] dev_hard_start_xmit+0x94/0x518 net/core/dev.c:3609 sch_direct_xmit+0x11c/0x1f0 net/sched/sch_generic.c:313 qdisc_restart net/sched/sch_generic.c:376 [inline] __qdisc_run+0x194/0x7f8 net/sched/sch_generic.c:384 qdisc_run include/net/pkt_sched.h:136 [inline] qdisc_run include/net/pkt_sched.h:128 [inline] __dev_xmit_skb net/core/dev.c:3795 [inline] __dev_queue_xmit+0x65c/0xf84 net/core/dev.c:4150 dev_queue_xmit+0x14/0x18 net/core/dev.c:4215 neigh_resolve_output net/core/neighbour.c:1491 [inline] neigh_resolve_output+0x170/0x228 net/core/neighbour.c:1471 neigh_output include/net/neighbour.h:510 [inline] ip6_finish_output2+0x2e4/0x9fc net/ipv6/ip6_output.c:117 __ip6_finish_output net/ipv6/ip6_output.c:182 [inline] __ip6_finish_output+0x164/0x3f8 net/ipv6/ip6_output.c:161 ip6_finish_output+0x2c/0xb0 net/ipv6/ip6_output.c:192 NF_HOOK_COND include/linux/netfilter.h:290 [inline] ip6_output+0x74/0x294 net/ipv6/ip6_output.c:215 dst_output include/net/dst.h:448 [inline] NF_HOOK include/linux/netfilter.h:301 [inline] NF_HOOK include/linux/netfilter.h:295 [inline] mld_sendpack+0x2a8/0x7e4 net/ipv6/mcast.c:1679 mld_send_cr net/ipv6/mcast.c:1975 [inline] mld_ifc_timer_expire+0x1e8/0x494 net/ipv6/mcast.c:2474 call_timer_fn+0xd0/0x570 kernel/time/timer.c:1431 expire_timers kernel/time/timer.c:1476 [inline] __run_timers kernel/time/timer.c:1745 [inline] run_timer_softirq+0x2e4/0x384 kernel/time/timer.c:1758 __do_softirq+0x204/0x7ac kernel/softirq.c:345 do_softirq_own_stack include/asm-generic/softirq_stack.h:10 [inline] invoke_softirq kernel/softirq.c:228 [inline] __irq_exit_rcu+0x1d8/0x200 kernel/softirq.c:422 irq_exit+0x10/0x3c kernel/softirq.c:446 __handle_domain_irq+0xb4/0x120 kernel/irq/irqdesc.c:692 handle_domain_irq include/linux/irqdesc.h:176 [inline] gic_handle_irq+0x84/0xac drivers/irqchip/irq-gic.c:370 __irq_svc+0x5c/0x94 arch/arm/kernel/entry-armv.S:205 debug_smp_processor_id+0x0/0x24 lib/smp_processor_id.c:53 rcu_read_lock_held_common kernel/rcu/update.c:108 [inline] rcu_read_lock_sched_held+0x24/0x7c kernel/rcu/update.c:123 trace_lock_acquire+0x24c/0x278 include/trace/events/lock.h:13 lock_acquire+0x3c/0x74 kernel/locking/lockdep.c:5481 rcu_lock_acquire include/linux/rcupdate.h:267 [inline] rcu_read_lock include/linux/rcupdate.h:656 [inline] avc_has_perm_noaudit+0x6c/0x260 security/selinux/avc.c:1150 selinux_inode_permission+0x140/0x220 security/selinux/hooks.c:3141 security_inode_permission+0x44/0x60 security/security.c:1268 inode_permission.part.0+0x5c/0x13c fs/namei.c:521 inode_permission fs/namei.c:494 [inline] may_lookup fs/namei.c:1652 [inline] link_path_walk.part.0+0xd4/0x38c fs/namei.c:2208 link_path_walk fs/namei.c:2189 [inline] path_lookupat+0x3c/0x1b8 fs/namei.c:2419 filename_lookup+0xa8/0x1a4 fs/namei.c:2453 user_path_at_empty+0x74/0x90 fs/namei.c:2733 do_readlinkat+0x5c/0x12c fs/stat.c:417 __do_sys_readlink fs/stat.c:450 [inline] sys_readlink+0x24/0x28 fs/stat.c:447 ret_fast_syscall+0x0/0x2c arch/arm/mm/proc-v7.S:64 0x7eaa4974 irq event stamp: 298277 hardirqs last enabled at (298277): [<802000d0>] no_work_pending+0x4/0x34 hardirqs last disabled at (298276): [<8020c9b8>] do_work_pending+0x9c/0x648 arch/arm/kernel/signal.c:676 softirqs last enabled at (298216): [<8020167c>] __do_softirq+0x584/0x7ac kernel/softirq.c:372 softirqs last disabled at (298201): [<8024dff4>] do_softirq_own_stack include/asm-generic/softirq_stack.h:10 [inline] softirqs last disabled at (298201): [<8024dff4>] invoke_softirq kernel/softirq.c:228 [inline] softirqs last disabled at (298201): [<8024dff4>] __irq_exit_rcu+0x1d8/0x200 kernel/softirq.c:422 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&(&pstats->syncp)->seq); lock(&(&pstats->syncp)->seq); *** DEADLOCK *** 1 lock held by udevd/4013: #0: 82b09c5c (rcu_read_lock){....}-{1:2}, at: sk_filter_trim_cap+0x54/0x434 net/core/filter.c:139 stack backtrace: CPU: 1 PID: 4013 Comm: udevd Not tainted 5.12.0-rc3-syzkaller #0 Hardware name: ARM-Versatile Express Backtrace: [<81802550>] (dump_backtrace) from [<818027c4>] (show_stack+0x18/0x1c arch/arm/kernel/traps.c:252) r7:00000080 r6:600d0093 r5:00000000 r4:82b58344 [<818027ac>] (show_stack) from [<81809e98>] (__dump_stack lib/dump_stack.c:79 [inline]) [<818027ac>] (show_stack) from [<81809e98>] (dump_stack+0xb8/0xe8 lib/dump_stack.c:120) [<81809de0>] (dump_stack) from [<81804a00>] (print_usage_bug.part.0+0x228/0x230 kernel/locking/lockdep.c:3806) r7:86bcb768 r6:81a0326c r5:830f96a8 r4:86bcb0c0 [<818047d8>] (print_usage_bug.part.0) from [<802bb1b8>] (print_usage_bug kernel/locking/lockdep.c:3776 [inline]) [<818047d8>] (print_usage_bug.part.0) from [<802bb1b8>] (valid_state kernel/locking/lockdep.c:3818 [inline]) [<818047d8>] (print_usage_bug.part.0) from [<802bb1b8>] (mark_lock_irq kernel/locking/lockdep.c:4021 [inline]) [<818047d8>] (print_usage_bug.part.0) from [<802bb1b8>] (mark_lock.part.0+0xc34/0x136c kernel/locking/lockdep.c:4478) r10:83278fe8 r9:82c6d748 r8:00000000 r7:82c6d2d4 r6:00000004 r5:86bcb768 r4:00000006 [<802ba584>] (mark_lock.part.0) from [<802bc644>] (mark_lock kernel/locking/lockdep.c:4442 [inline]) [<802ba584>] (mark_lock.part.0) from [<802bc644>] (mark_usage kernel/locking/lockdep.c:4391 [inline]) [<802ba584>] (mark_lock.part.0) from [<802bc644>] (__lock_acquire+0x9bc/0x3318 kernel/locking/lockdep.c:4854) r10:86bcb768 r9:86bcb0c0 r8:00000001 r7:00040000 r6:0000075a r5:830f96a8 r4:00000000 [<802bbc88>] (__lock_acquire) from [<802bfb90>] (lock_acquire.part.0+0xf0/0x41c kernel/locking/lockdep.c:5510) r10:00000000 r9:600d0013 r8:00000000 r7:00000000 r6:828a2680 r5:828a2680 r4:861e5bc8 [<802bfaa0>] (lock_acquire.part.0) from [<802bff28>] (lock_acquire+0x6c/0x74 kernel/locking/lockdep.c:5483) r10:8146137c r9:00000000 r8:00000001 r7:00000000 r6:00000000 r5:00000000 r4:ff7c9dec [<802bfebc>] (lock_acquire) from [<81381eb4>] (do_write_seqcount_begin_nested include/linux/seqlock.h:520 [inline]) [<802bfebc>] (lock_acquire) from [<81381eb4>] (do_write_seqcount_begin include/linux/seqlock.h:545 [inline]) [<802bfebc>] (lock_acquire) from [<81381eb4>] (u64_stats_update_begin include/linux/u64_stats_sync.h:129 [inline]) [<802bfebc>] (lock_acquire) from [<81381eb4>] (__bpf_prog_run_save_cb include/linux/filter.h:727 [inline]) [<802bfebc>] (lock_acquire) from [<81381eb4>] (bpf_prog_run_save_cb include/linux/filter.h:741 [inline]) [<802bfebc>] (lock_acquire) from [<81381eb4>] (sk_filter_trim_cap+0x26c/0x434 net/core/filter.c:149) r10:a4095dd0 r9:ff7c9dd0 r8:e44be000 r7:8146137c r6:00000001 r5:8611ba80 r4:00000000 [<81381c48>] (sk_filter_trim_cap) from [<8146137c>] (sk_filter include/linux/filter.h:867 [inline]) [<81381c48>] (sk_filter_trim_cap) from [<8146137c>] (do_one_broadcast net/netlink/af_netlink.c:1468 [inline]) [<81381c48>] (sk_filter_trim_cap) from [<8146137c>] (netlink_broadcast_filtered+0x27c/0x4fc net/netlink/af_netlink.c:1520) r10:00000001 r9:833d6b1c r8:00000000 r7:8572f864 r6:8611ba80 r5:8698d800 r4:8572f800 [<81461100>] (netlink_broadcast_filtered) from [<81463e60>] (netlink_broadcast net/netlink/af_netlink.c:1544 [inline]) [<81461100>] (netlink_broadcast_filtered) from [<81463e60>] (netlink_sendmsg+0x3d0/0x478 net/netlink/af_netlink.c:1925) r10:00000000 r9:00000002 r8:8698d800 r7:000000b7 r6:8611b900 r5:861e5f50 r4:86aa3000 [<81463a90>] (netlink_sendmsg) from [<81321f54>] (sock_sendmsg_nosec net/socket.c:654 [inline]) [<81463a90>] (netlink_sendmsg) from [<81321f54>] (sock_sendmsg+0x3c/0x4c net/socket.c:674) r10:00000000 r9:861e5dd4 r8:00000000 r7:86570000 r6:00000000 r5:86570000 r4:861e5f50 [<81321f18>] (sock_sendmsg) from [<813234d0>] (____sys_sendmsg+0x230/0x29c net/socket.c:2350) r5:00000040 r4:861e5f50 [<813232a0>] (____sys_sendmsg) from [<8132549c>] (___sys_sendmsg+0xac/0xe4 net/socket.c:2404) r10:00000128 r9:861e4000 r8:00000000 r7:00000000 r6:86570000 r5:861e5f50 r4:00000000 [<813253f0>] (___sys_sendmsg) from [<81325684>] (__sys_sendmsg net/socket.c:2433 [inline]) [<813253f0>] (___sys_sendmsg) from [<81325684>] (__do_sys_sendmsg net/socket.c:2442 [inline]) [<813253f0>] (___sys_sendmsg) from [<81325684>] (sys_sendmsg+0x58/0xa0 net/socket.c:2440) r8:80200224 r7:00000128 r6:00000000 r5:7eaa541c r4:86570000 [<8132562c>] (sys_sendmsg) from [<80200060>] (ret_fast_syscall+0x0/0x2c arch/arm/mm/proc-v7.S:64) Exception stack(0x861e5fa8 to 0x861e5ff0) 5fa0: 00000000 00000000 0000000c 7eaa541c 00000000 00000000 5fc0: 00000000 00000000 76fbf840 00000128 00000000 0000008f 7eaa541c 000563f8 5fe0: 00056110 7eaa53e0 00036cec 76c9bf44 r6:76fbf840 r5:00000000 r4:00000000 Fixes: 492ecee892c2 ("bpf: enable program stats") Signed-off-by: Eric Dumazet Cc: Alexei Starovoitov Cc: Daniel Borkmann Cc: Andrii Nakryiko Reported-by: syzbot --- include/linux/filter.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 47f80adbe744769ad4372e50aa8d2b6a767deb12..2fffe9cc50f946f24f4f78b59902bad91f910e5b 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -612,13 +612,14 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog, if (static_branch_unlikely(&bpf_stats_enabled_key)) { struct bpf_prog_stats *stats; u64 start = sched_clock(); + unsigned long flags; ret = dfunc(ctx, prog->insnsi, prog->bpf_func); stats = this_cpu_ptr(prog->stats); - u64_stats_update_begin(&stats->syncp); + flags = u64_stats_update_begin_irqsave(&stats->syncp); stats->cnt++; stats->nsecs += sched_clock() - start; - u64_stats_update_end(&stats->syncp); + u64_stats_update_end_irqrestore(&stats->syncp, flags); } else { ret = dfunc(ctx, prog->insnsi, prog->bpf_func); } From patchwork Tue Oct 26 21:41:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 12585721 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 41C52C43217 for ; Tue, 26 Oct 2021 21:41:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 270F1610A6 for ; Tue, 26 Oct 2021 21:41:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236136AbhJZVoI (ORCPT ); Tue, 26 Oct 2021 17:44:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42544 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235793AbhJZVoD (ORCPT ); Tue, 26 Oct 2021 17:44:03 -0400 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 87380C061745; Tue, 26 Oct 2021 14:41:39 -0700 (PDT) Received: by mail-pf1-x42b.google.com with SMTP id 187so702454pfc.10; Tue, 26 Oct 2021 14:41:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=zYiUZsuAt0d8/CfnIpvY7tOtAIepLvDP7A9G8sy+glI=; b=RpAAO7kXo2VfE/3Qae1nBeHjIAfZKzj87hv5ekYTU08IHeE0+PNsLKEaBcYVlyGeXA vleVVRIHuMlaWPP6sYMp5Iq041VK1ZcLEs4IV73rs/JKzGoHvSsibNzc5SQsSRG3jd2h Oh0EZui9U5FHftQ55GqLbptge0jK6cLN5CrT4DXdxHoecYNMYFOdLaCzT/W3Qf7tM+Yz FAQjuJjN0MMJshJCjs4EvYFGWbcjAYjHH9hmDr1KhOB/cRGRtUjtVouJSxdbSvQAlGjC 7tdCuM1nAD8tY07rNXZmfMwmvo69aA9dJ5E2x5VdYDgABGkYQkFrDq7Jqdlhf9ZW5080 xd0w== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=zYiUZsuAt0d8/CfnIpvY7tOtAIepLvDP7A9G8sy+glI=; b=mKzCZMCR+REql3+wKCptU/0BGHnhtdKh+F2KKWatDL24+nsqxRUVDxoFShYWMXHfD+ xTC/KlvU+gaEriXE2gUhRMW1RxHJpGVaQZ4pcW/S93cyfgfFPh8fo8qzazWUXMzSfufT Q7As4HBY5nX2hQP5EUUuV3Ly5+a7m4z1nSi2S4d7WTbP/fuvFoPDOa0UO0rC1ZUkBG6o 8MjE1AsBwnvDlwyUhfMC6bOEJOrotyx1+63aueM5PmQmEzFhATMYgtCBlzzHaL6RueWf HPkQPcQTAXk+x8zWqTUIOrM3Ca8MgVmnQ5GGIPpIMK4987+v57rheCVX/63lYsXe/WkB 0EjQ== X-Gm-Message-State: AOAM530cVwML6ojGQgEv0NwaA3QxumATF1RjC8fXIq4dfgyJfY7BdqW4 +ajfOTTII75n84Xa+YSLLNo= X-Google-Smtp-Source: ABdhPJz2Up0zxfomMxEbV1N+Dcq61rPRBDMwTSH1T4iXe6QCV4CvFS9FSHnIrsSH1WzQF4C2uoJ09g== X-Received: by 2002:a05:6a00:13a3:b0:47c:126:3596 with SMTP id t35-20020a056a0013a300b0047c01263596mr11906959pfg.5.1635284499143; Tue, 26 Oct 2021 14:41:39 -0700 (PDT) Received: from edumazet1.svl.corp.google.com ([2620:15c:2c4:201:207c:7102:7bd7:80eb]) by smtp.gmail.com with ESMTPSA id g22sm6495400pfj.181.2021.10.26.14.41.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Oct 2021 14:41:38 -0700 (PDT) From: Eric Dumazet To: Alexei Starovoitov , Daniel Borkmann Cc: "David S . Miller" , netdev , Eric Dumazet , Eric Dumazet , bpf Subject: [PATCH V2 bpf-next 2/3] bpf: fixes possible race in update_prog_stats() for 32bit arches Date: Tue, 26 Oct 2021 14:41:32 -0700 Message-Id: <20211026214133.3114279-3-eric.dumazet@gmail.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog In-Reply-To: <20211026214133.3114279-1-eric.dumazet@gmail.com> References: <20211026214133.3114279-1-eric.dumazet@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net From: Eric Dumazet It seems update_prog_stats() suffers from same issue fixed in the prior patch: As it can run while interrupts are enabled, it could be re-entered and the u64_stats syncp could be mangled. Fixes: fec56f5890d9 ("bpf: Introduce BPF trampoline") Signed-off-by: Eric Dumazet Cc: Alexei Starovoitov Cc: Daniel Borkmann --- kernel/bpf/trampoline.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index 39eaaff81953da6006702635fd67d08dd4a7daa2..e5963de368ed34874414d097bc6614ff7e168dd3 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -586,11 +586,13 @@ static void notrace update_prog_stats(struct bpf_prog *prog, * Hence check that 'start' is valid. */ start > NO_START_TIME) { + unsigned long flags; + stats = this_cpu_ptr(prog->stats); - u64_stats_update_begin(&stats->syncp); + flags = u64_stats_update_begin_irqsave(&stats->syncp); stats->cnt++; stats->nsecs += sched_clock() - start; - u64_stats_update_end(&stats->syncp); + u64_stats_update_end_irqrestore(&stats->syncp, flags); } } From patchwork Tue Oct 26 21:41:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 12585723 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A751DC4321E for ; Tue, 26 Oct 2021 21:41:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8DB99610A6 for ; Tue, 26 Oct 2021 21:41:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237232AbhJZVoI (ORCPT ); Tue, 26 Oct 2021 17:44:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42554 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235366AbhJZVoG (ORCPT ); Tue, 26 Oct 2021 17:44:06 -0400 Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8589C061767; Tue, 26 Oct 2021 14:41:41 -0700 (PDT) Received: by mail-pf1-x429.google.com with SMTP id 127so747561pfu.1; Tue, 26 Oct 2021 14:41:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=uRZB2rF8sHKDVMLWBScuHHaIQnj2j20RrdT9H9xm/3c=; b=noawh3oskqNWSO7UlEcKGWOuv5x7kOu0dOf0KYmpCo4aNAQY9bA8cf3qNeiyEKbd4o 9ZUCI9WaNRN4I/5VRD8fEZWbMR5WCLD5zTAWWhBosUV6Ckyo+RpZxRxd5Sv3TjP35j7j j+l+0NHXZcP/3kP7Sv2AA7qO87Cqt/D6QqPg08dz1qgNC1qjHBmBEAK39SxVd8mo4NH2 Yv1oBg58DOPObFh7/v7FaxRMnlW+OXllzLZ9WSH4NgUPM3O6SfKNfYIXlAvlXgPdEgar rNsxXXEYzXfOpD4SxY0Do2KfMk7IN/fPbqln8oo2PZef0TasNMpvkEvtNBwScKM/bS+y ltCA== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=uRZB2rF8sHKDVMLWBScuHHaIQnj2j20RrdT9H9xm/3c=; b=gTIPq+qiHhGs2AAvEmh6Co244NYuNNtXpl/IbU/dUPdpsDYba2qFJ1iFuOXbcvrNa3 5fLntfrp9DBaeCq01N0XGW6VMa4z/QAcP+r89xCW9kC/bSDi6wskrz0QvtpgRCTPhKFd +GcV5qA2msR95YeIcfXeyxsekK+ZbtWncRyVKz9VYHyBGjp/I8qJzWAo8IFnT0Gcmxn2 sJV3FLQfE50WRZxzEUecOzBayOGyQkLzVNXGx6XPLxNqNhMR98gxZPzJjBfyrBUVdiJR bEXEU5r5a+6Tai49fCYikkodrCvCo7KwltUOtHqXJYuC3x4UP+JVAWGgHvS4ObnVB5a0 NDYw== X-Gm-Message-State: AOAM533EoNepCtQKbGeySI//UI0gj4BRqlgcbraPxwYrW/i3DEBX6cZn l6+CC/V24kAVhktKDBkQnX4= X-Google-Smtp-Source: ABdhPJw/7VohMZ4MaDTTWlIoH+qGitBHyuWxRhUa7MzK2AlTBXQ0jXGavGJdaqw0nf/lMdM82PZbZg== X-Received: by 2002:a05:6a00:1a01:b0:47b:ae61:9bd1 with SMTP id g1-20020a056a001a0100b0047bae619bd1mr28173461pfv.0.1635284500389; Tue, 26 Oct 2021 14:41:40 -0700 (PDT) Received: from edumazet1.svl.corp.google.com ([2620:15c:2c4:201:207c:7102:7bd7:80eb]) by smtp.gmail.com with ESMTPSA id g22sm6495400pfj.181.2021.10.26.14.41.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Oct 2021 14:41:40 -0700 (PDT) From: Eric Dumazet To: Alexei Starovoitov , Daniel Borkmann Cc: "David S . Miller" , netdev , Eric Dumazet , Eric Dumazet , bpf Subject: [PATCH V2 bpf-next 3/3] bpf: use u64_stats_t in struct bpf_prog_stats Date: Tue, 26 Oct 2021 14:41:33 -0700 Message-Id: <20211026214133.3114279-4-eric.dumazet@gmail.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog In-Reply-To: <20211026214133.3114279-1-eric.dumazet@gmail.com> References: <20211026214133.3114279-1-eric.dumazet@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: bpf@iogearbox.net From: Eric Dumazet Commit 316580b69d0a ("u64_stats: provide u64_stats_t type") fixed possible load/store tearing on 64bit arches. For instance the following C code stats->nsecs += sched_clock() - start; Could be rightfully implemented like this by a compiler, confusing concurrent readers a lot: stats->nsecs += sched_clock(); // arbitrary delay stats->nsecs -= start; Signed-off-by: Eric Dumazet Cc: Alexei Starovoitov Cc: Daniel Borkmann --- include/linux/filter.h | 10 +++++----- kernel/bpf/syscall.c | 18 ++++++++++++------ kernel/bpf/trampoline.c | 6 +++--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 2fffe9cc50f946f24f4f78b59902bad91f910e5b..9782e32458522273b314579e62c6215ebb07a617 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -553,9 +553,9 @@ struct bpf_binary_header { }; struct bpf_prog_stats { - u64 cnt; - u64 nsecs; - u64 misses; + u64_stats_t cnt; + u64_stats_t nsecs; + u64_stats_t misses; struct u64_stats_sync syncp; } __aligned(2 * sizeof(u64)); @@ -617,8 +617,8 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog, ret = dfunc(ctx, prog->insnsi, prog->bpf_func); stats = this_cpu_ptr(prog->stats); flags = u64_stats_update_begin_irqsave(&stats->syncp); - stats->cnt++; - stats->nsecs += sched_clock() - start; + u64_stats_inc(&stats->cnt); + u64_stats_add(&stats->nsecs, sched_clock() - start); u64_stats_update_end_irqrestore(&stats->syncp, flags); } else { ret = dfunc(ctx, prog->insnsi, prog->bpf_func); diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 4e50c0bfdb7d38e1c929a499c0688f59b9caf618..7a147c6eb30b2b771539cce642a359bd4dec66f8 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1804,8 +1804,14 @@ static int bpf_prog_release(struct inode *inode, struct file *filp) return 0; } +struct bpf_prog_kstats { + u64 nsecs; + u64 cnt; + u64 misses; +}; + static void bpf_prog_get_stats(const struct bpf_prog *prog, - struct bpf_prog_stats *stats) + struct bpf_prog_kstats *stats) { u64 nsecs = 0, cnt = 0, misses = 0; int cpu; @@ -1818,9 +1824,9 @@ static void bpf_prog_get_stats(const struct bpf_prog *prog, st = per_cpu_ptr(prog->stats, cpu); do { start = u64_stats_fetch_begin_irq(&st->syncp); - tnsecs = st->nsecs; - tcnt = st->cnt; - tmisses = st->misses; + tnsecs = u64_stats_read(&st->nsecs); + tcnt = u64_stats_read(&st->cnt); + tmisses = u64_stats_read(&st->misses); } while (u64_stats_fetch_retry_irq(&st->syncp, start)); nsecs += tnsecs; cnt += tcnt; @@ -1836,7 +1842,7 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp) { const struct bpf_prog *prog = filp->private_data; char prog_tag[sizeof(prog->tag) * 2 + 1] = { }; - struct bpf_prog_stats stats; + struct bpf_prog_kstats stats; bpf_prog_get_stats(prog, &stats); bin2hex(prog_tag, prog->tag, sizeof(prog->tag)); @@ -3575,7 +3581,7 @@ static int bpf_prog_get_info_by_fd(struct file *file, struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info); struct bpf_prog_info info; u32 info_len = attr->info.info_len; - struct bpf_prog_stats stats; + struct bpf_prog_kstats stats; char __user *uinsns; u32 ulen; int err; diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index e5963de368ed34874414d097bc6614ff7e168dd3..e98de5e73ba59f756480cc5f962849be1859751b 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -545,7 +545,7 @@ static void notrace inc_misses_counter(struct bpf_prog *prog) stats = this_cpu_ptr(prog->stats); u64_stats_update_begin(&stats->syncp); - stats->misses++; + u64_stats_inc(&stats->misses); u64_stats_update_end(&stats->syncp); } @@ -590,8 +590,8 @@ static void notrace update_prog_stats(struct bpf_prog *prog, stats = this_cpu_ptr(prog->stats); flags = u64_stats_update_begin_irqsave(&stats->syncp); - stats->cnt++; - stats->nsecs += sched_clock() - start; + u64_stats_inc(&stats->cnt); + u64_stats_add(&stats->nsecs, sched_clock() - start); u64_stats_update_end_irqrestore(&stats->syncp, flags); } }