From patchwork Sun Jan 8 16:39:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 13092585 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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 69ECBC53210 for ; Sun, 8 Jan 2023 16:40:56 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pEYhy-0000wE-KF; Sun, 08 Jan 2023 11:39:42 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pEYhx-0000tI-66 for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:41 -0500 Received: from mail-qv1-xf32.google.com ([2607:f8b0:4864:20::f32]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pEYhv-0002ei-Dp for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:40 -0500 Received: by mail-qv1-xf32.google.com with SMTP id o17so4631600qvn.4 for ; Sun, 08 Jan 2023 08:39:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=2355NoDCE5lj81ub3moTL5Yl3xuNfaymEqLI/iKbB2c=; b=M54ZFBoM26eP7s4T6OEu1TrNnE/34XHoPdfcM2I8VCYh6Sa1+TXrcbqiCKNpy+PjtS /txv9JoXJ3j3gRrDPqtHhs7uSeWfrzry/qwihZytk1G7c9ZAyjCmJNmETSD9QL5BRj+I fTOB6TLx8Trfcjyk3RvIfXIUhBxhJHaNx+WAsfwyuLnrIstHpvDFzXxmjaaCUs0V9/Gt lsKLNSmiXNvvhZX5PXRYVIbs7jY4Hj6/HTCrw6LM3HQGFaJFiN5l4lVmJw8DwzBfGOFg DhaR3XMtAQclBp7eQs678QK07ijwrG8ZJK/W/iP6mfXqt2srhEXW0zFlJ2lRux91zYQu Ud9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2355NoDCE5lj81ub3moTL5Yl3xuNfaymEqLI/iKbB2c=; b=lisqOLFUZAGx1wwZeszNva9eXupCnOgchyqyPfODsaQAh69KamzT7zCfTJLey8j6jO croENMEiAUX6QOoWQDiljNCnnPpdwRuwpzC6zE/VdMwiRHUcg5sRVR0uOJ+9KQ+Wo6bt P8rbEdFUvzYGps5TECc+g74MR7piosc7qdTRhElgfk8qwCcjmi0GeOAIoqBo7QgGXiUq kksKz1lGMXNOrfmpWVgT8bUUvhuuijZkAhVWxO6kn9C5FYofsFMgXF2peO/HiGTnxL// kTp3wuTOp2Rd+tvhqrB9gyOni2O7XtLCzfflBYaxc+2cgoWoHugNmgY5rEDanUsS2TfO W7RQ== X-Gm-Message-State: AFqh2krcG+CQyH0WmyYx3BVR9cL6sc6C9LElsj2vPJaq7qa4hcbFsi1U KG5CYDU6XlXLqSsjqO2gQ1WEcNyvdqkrwMhtR73mJA== X-Google-Smtp-Source: AMrXdXvRsyCtLTP3Q4yeOHrsNMM6HRWGMKLr7/8nmRpxT0TFQC/ufj/152YsgDHtXKeLWN8pG2nT/Q== X-Received: by 2002:a05:6214:5e04:b0:4bb:7a71:c35f with SMTP id li4-20020a0562145e0400b004bb7a71c35fmr86857142qvb.46.1673195978154; Sun, 08 Jan 2023 08:39:38 -0800 (PST) Received: from localhost ([185.156.46.146]) by smtp.gmail.com with ESMTPSA id dm42-20020a05620a1d6a00b00705bac8891esm92429qkb.40.2023.01.08.08.39.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Jan 2023 08:39:37 -0800 (PST) From: Emilio Cota To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , Paolo Bonzini , Richard Henderson , Emilio Cota Subject: [PATCH 1/4] cpu: free cpu->tb_jmp_cache with RCU Date: Sun, 8 Jan 2023 11:39:02 -0500 Message-Id: <20230108163905.59601-2-cota@braap.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230108163905.59601-1-cota@braap.org> References: <20230108163905.59601-1-cota@braap.org> MIME-Version: 1.0 Received-SPF: softfail client-ip=2607:f8b0:4864:20::f32; envelope-from=cota@braap.org; helo=mail-qv1-xf32.google.com X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Fixes the appended use-after-free. The root cause is that during tb invalidation we use CPU_FOREACH, and therefore to safely free a vCPU we must wait for an RCU grace period to elapse. $ x86_64-linux-user/qemu-x86_64 tests/tcg/x86_64-linux-user/munmap-pthread ================================================================= ==1800604==ERROR: AddressSanitizer: heap-use-after-free on address 0x62d0005f7418 at pc 0x5593da6704eb bp 0x7f4961a7ac70 sp 0x7f4961a7ac60 READ of size 8 at 0x62d0005f7418 thread T2 #0 0x5593da6704ea in tb_jmp_cache_inval_tb ../accel/tcg/tb-maint.c:244 #1 0x5593da6704ea in do_tb_phys_invalidate ../accel/tcg/tb-maint.c:290 #2 0x5593da670631 in tb_phys_invalidate__locked ../accel/tcg/tb-maint.c:306 #3 0x5593da670631 in tb_invalidate_phys_page_range__locked ../accel/tcg/tb-maint.c:542 #4 0x5593da67106d in tb_invalidate_phys_range ../accel/tcg/tb-maint.c:614 #5 0x5593da6a64d4 in target_munmap ../linux-user/mmap.c:766 #6 0x5593da6dba05 in do_syscall1 ../linux-user/syscall.c:10105 #7 0x5593da6f564c in do_syscall ../linux-user/syscall.c:13329 #8 0x5593da49e80c in cpu_loop ../linux-user/x86_64/../i386/cpu_loop.c:233 #9 0x5593da6be28c in clone_func ../linux-user/syscall.c:6633 #10 0x7f496231cb42 in start_thread nptl/pthread_create.c:442 #11 0x7f49623ae9ff (/lib/x86_64-linux-gnu/libc.so.6+0x1269ff) 0x62d0005f7418 is located 28696 bytes inside of 32768-byte region [0x62d0005f0400,0x62d0005f8400) freed by thread T148 here: #0 0x7f49627b6460 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0x5593da5ac057 in cpu_exec_unrealizefn ../cpu.c:180 #2 0x5593da81f851 (/home/cota/src/qemu/build/qemu-x86_64+0x484851) Signed-off-by: Emilio Cota --- accel/tcg/cpu-exec.c | 1 - cpu.c | 7 +++++++ include/hw/core/cpu.h | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 356fe348de..ca95d21528 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -1070,7 +1070,6 @@ void tcg_exec_unrealizefn(CPUState *cpu) #endif /* !CONFIG_USER_ONLY */ tlb_destroy(cpu); - g_free(cpu->tb_jmp_cache); } #ifndef CONFIG_USER_ONLY diff --git a/cpu.c b/cpu.c index 4a7d865427..564200559f 100644 --- a/cpu.c +++ b/cpu.c @@ -164,6 +164,12 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp) #endif /* CONFIG_USER_ONLY */ } +static void cpu_free_rcu(CPUState *cpu) +{ + /* .tb_jmp_cache is NULL except under TCG */ + g_free(cpu->tb_jmp_cache); +} + void cpu_exec_unrealizefn(CPUState *cpu) { #ifndef CONFIG_USER_ONLY @@ -181,6 +187,7 @@ void cpu_exec_unrealizefn(CPUState *cpu) } cpu_list_remove(cpu); + call_rcu(cpu, cpu_free_rcu, rcu); } /* diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 8830546121..9fefad5656 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -266,6 +266,7 @@ struct qemu_work_item; /** * CPUState: + * @rcu: Used for safe deferred memory reclamation. * @cpu_index: CPU index (informative). * @cluster_index: Identifies which cluster this CPU is in. * For boards which don't define clusters or for "loose" CPUs not assigned @@ -321,6 +322,8 @@ struct qemu_work_item; * State of one CPU core or thread. */ struct CPUState { + struct rcu_head rcu; + /*< private >*/ DeviceState parent_obj; /* cache to avoid expensive CPU_GET_CLASS */ From patchwork Sun Jan 8 16:39:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 13092584 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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3ECD5C54EBD for ; Sun, 8 Jan 2023 16:40:26 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pEYi0-00010H-BG; Sun, 08 Jan 2023 11:39:44 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pEYhz-0000y5-Bp for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:43 -0500 Received: from mail-qt1-x82f.google.com ([2607:f8b0:4864:20::82f]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pEYhx-0002fI-UI for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:43 -0500 Received: by mail-qt1-x82f.google.com with SMTP id h21so6022182qta.12 for ; Sun, 08 Jan 2023 08:39:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=BaGbKhSnqhHFWAmCitnff4clI2gayRmQn8Im1y8M/aY=; b=srq5Bv8vefDYwJ+7NtFDMUaRAT6JfQgUwG9ZSOPpWsFQV6wqqWLkSIQQ29SM9ohsc9 iGlz3kas8F02PRhJ61w0VOKEElNImG5UW9GEgRC3qXkgulk5zcKe5aaOj7TZq7vpwJfx TTqk9ny+QmqJxIAjiPGF7RZ3OhWLRCb4XuCUv0N4F4wRfKsPUy2YGJHTbDYNMt9HMfxN E3pXM+/ukrV0IepAFRbqVAelIFGqnaiQfF7M7gKwQz1Em8mLPVY9iQlpfTIxxbc6xzmw pYuwEMr3d/W3WMn8wtZeEo+rhe8NyM9CZeG/yKUpp5vbPvptmbPxkGjb6J/+WGyeTgQ3 Mwjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BaGbKhSnqhHFWAmCitnff4clI2gayRmQn8Im1y8M/aY=; b=A4BAw6rJ5Mfvn5qN9+arIt7+31GnEyJbYu6BNygpn3HrnnZ3BPZ+F7GVkQpnkasT+O 1cqUvhrzBmyzxswZN2dld8TYVXTjMhmObA9cdKFfjGhBZMdQY5XremgHnhs9sOX3gAB8 PIcDvXpWNdSSdCv28NpaXXleLX62UXMFJU/HJx6B1YCCCDcnAUtQWYrx/H8rud/2hhP4 YOrbwIzDqMuBm4nzbFH+rYr+aw9V95noVhn5IdGMt3jGqU8fyJSmtmDEamWg+qbpYghA /Vl5zBi8FP4vLRn0HguDZlr6GBWaI0zt34GPe6utr83F+KSlJbupiKwfiqhuz2UtpBM5 2u+g== X-Gm-Message-State: AFqh2koWhbNf0jyRX0VPYdn1IZScYkpsWmUlCrOfjwsfh85NVX2MiOdd mW5RoYMhst0f8N9gvnKUUlyUvpeRU4+JCn1bawVv6Q== X-Google-Smtp-Source: AMrXdXsvk9lwNfLH6NwiVVHdumvaQrF9GljS/HaIs+e2cWGdZDLjmwYk4N9BvhVZL3PiWsOxnGgCPA== X-Received: by 2002:a05:622a:a18:b0:3a5:8b:7719 with SMTP id bv24-20020a05622a0a1800b003a5008b7719mr84161270qtb.65.1673195980684; Sun, 08 Jan 2023 08:39:40 -0800 (PST) Received: from localhost ([185.156.46.146]) by smtp.gmail.com with ESMTPSA id d7-20020ac80607000000b003a7ec97c882sm3406995qth.6.2023.01.08.08.39.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Jan 2023 08:39:40 -0800 (PST) From: Emilio Cota To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , Paolo Bonzini , Richard Henderson , Emilio Cota Subject: [PATCH 2/4] util/qht: add missing atomic_set(hashes[i]) Date: Sun, 8 Jan 2023 11:39:03 -0500 Message-Id: <20230108163905.59601-3-cota@braap.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230108163905.59601-1-cota@braap.org> References: <20230108163905.59601-1-cota@braap.org> MIME-Version: 1.0 Received-SPF: softfail client-ip=2607:f8b0:4864:20::82f; envelope-from=cota@braap.org; helo=mail-qt1-x82f.google.com X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org We forgot to add this one in "a890643958 util/qht: atomically set b->hashes". Detected with tsan. Signed-off-by: Emilio Cota Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- util/qht.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/qht.c b/util/qht.c index 065fc501f4..15866299e6 100644 --- a/util/qht.c +++ b/util/qht.c @@ -688,7 +688,7 @@ static inline void qht_bucket_remove_entry(struct qht_bucket *orig, int pos) int i; if (qht_entry_is_last(orig, pos)) { - orig->hashes[pos] = 0; + qatomic_set(&orig->hashes[pos], 0); qatomic_set(&orig->pointers[pos], NULL); return; } From patchwork Sun Jan 8 16:39:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 13092586 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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7148FC53210 for ; Sun, 8 Jan 2023 16:41:13 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pEYi5-0001B6-6l; Sun, 08 Jan 2023 11:39:49 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pEYi3-00016x-2W for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:47 -0500 Received: from mail-qt1-x832.google.com ([2607:f8b0:4864:20::832]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pEYi1-0002gD-03 for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:46 -0500 Received: by mail-qt1-x832.google.com with SMTP id z12so6120187qtv.5 for ; Sun, 08 Jan 2023 08:39:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=V3RSwsC9DODQ8KdtQXM9KFMTnGOhn8xhBBofjHmybpg=; b=zr6hxVCbhSoMKpEjUYEghgVzN5joDWxq7SUiZmxPp1+eGKYVySl7m0WdQCwTKWNjiU PpvEjIaAAcG8tpOlCaZACDH3Oj8JaP46Gz3JYmRgvdcgRSN+nnyVvZHmT2bVqGdfyfaM YwQKJyYf631bw/ARbtUh5kOI2c/IFffndgGtjq4jIRN20yTgDngfWuAjbD2TGEsTqtxL BcoHtsWGoQZyLvmp4IfaogvXYpbx+B+9cbNQraUANaf2BdBdN0CTTn2CajJWwunix9en 5A3ylR33mDFsB+TXbf6y4pdS7ByzF6Ii+AEoXPhS0r/ZxIYuYYbOK/nYBvXEQ+a0YSpa vpJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=V3RSwsC9DODQ8KdtQXM9KFMTnGOhn8xhBBofjHmybpg=; b=HiK3U0GDtaDVhpwaoFbeNwgZ+pyrWU7GZMpgHGmJdxOVswML0Nh648p0x5fWW+ypRx Vnt+Iff6tDCaTSlb3aTaQ4GBBEF3xfmUT3mkHnEONqMsHVsWI2ClwcIdvLxyGbOpxoJn omDYYkgYvDY9tgdg65gJewJ8d09IAeeXuL2+4URe0WRGbetG1/LxGgyjz1NfpmdaU9Br HPxVjHaJAO9K2g/s4PjzVN46GAMuV7qs5Ha9tZez798x6iFC8bS192/qTJo3rdy4Vy96 DFnXjAheuAKlLmJQiRYnv/FThyImcct6aDgDtNXiUOOo5qlELtxibSO4P3fszWfXgwwN eh/w== X-Gm-Message-State: AFqh2kqQbMMPtgr0nu/15WHt6Ac9YHLtvomIsrO32/D9Ea+PA3sAeGUU W61XukLQSIkhj4NyMIO9hvHTtYrwsT8LAFmfCEVJwA== X-Google-Smtp-Source: AMrXdXs+zSt53g6XLnquMY6KA1w83SWhTuNGt3HGOih332tX/iBN+BFWgu4d3dapVCDppomY23GI8A== X-Received: by 2002:ac8:6049:0:b0:3a7:f5fc:7cad with SMTP id k9-20020ac86049000000b003a7f5fc7cadmr82791650qtm.62.1673195983675; Sun, 08 Jan 2023 08:39:43 -0800 (PST) Received: from localhost ([185.156.46.146]) by smtp.gmail.com with ESMTPSA id bz27-20020a05622a1e9b00b003a9902d4150sm3421096qtb.82.2023.01.08.08.39.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Jan 2023 08:39:43 -0800 (PST) From: Emilio Cota To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , Paolo Bonzini , Richard Henderson , Emilio Cota Subject: [PATCH 3/4] util/qht: use striped locks under TSAN Date: Sun, 8 Jan 2023 11:39:04 -0500 Message-Id: <20230108163905.59601-4-cota@braap.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230108163905.59601-1-cota@braap.org> References: <20230108163905.59601-1-cota@braap.org> MIME-Version: 1.0 Received-SPF: softfail client-ip=2607:f8b0:4864:20::832; envelope-from=cota@braap.org; helo=mail-qt1-x832.google.com X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Fixes this tsan crash, easy to reproduce with any large enough program: $ tests/unit/test-qht 1..2 ThreadSanitizer: CHECK failed: sanitizer_deadlock_detector.h:67 "((n_all_locks_)) < (((sizeof(all_locks_with_contexts_)/sizeof((all_locks_with_contexts_)[0]))))" (0x40, 0x40) (tid=1821568) #0 __tsan::CheckUnwind() ../../../../src/libsanitizer/tsan/tsan_rtl.cpp:353 (libtsan.so.2+0x90034) #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xca555) #2 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:67 (libtsan.so.2+0xb3616) #3 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:59 (libtsan.so.2+0xb3616) #4 __sanitizer::DeadlockDetector<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector > >::onLockAfter(__sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector > >*, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:216 (libtsan.so.2+0xb3616) #5 __sanitizer::DD::MutexAfterLock(__sanitizer::DDCallback*, __sanitizer::DDMutex*, bool, bool) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector1.cpp:169 (libtsan.so.2+0xb3616) #6 __tsan::MutexPostLock(__tsan::ThreadState*, unsigned long, unsigned long, unsigned int, int) ../../../../src/libsanitizer/tsan/tsan_rtl_mutex.cpp:200 (libtsan.so.2+0xa3382) #7 __tsan_mutex_post_lock ../../../../src/libsanitizer/tsan/tsan_interface_ann.cpp:384 (libtsan.so.2+0x76bc3) #8 qemu_spin_lock /home/cota/src/qemu/include/qemu/thread.h:259 (test-qht+0x44a97) #9 qht_map_lock_buckets ../util/qht.c:253 (test-qht+0x44a97) #10 do_qht_iter ../util/qht.c:809 (test-qht+0x45f33) #11 qht_iter ../util/qht.c:821 (test-qht+0x45f33) #12 iter_check ../tests/unit/test-qht.c:121 (test-qht+0xe473) #13 qht_do_test ../tests/unit/test-qht.c:202 (test-qht+0xe473) #14 qht_test ../tests/unit/test-qht.c:240 (test-qht+0xe7c1) #15 test_default ../tests/unit/test-qht.c:246 (test-qht+0xe828) #16 (libglib-2.0.so.0+0x7daed) #17 (libglib-2.0.so.0+0x7d80a) #18 (libglib-2.0.so.0+0x7d80a) #19 g_test_run_suite (libglib-2.0.so.0+0x7dfe9) #20 g_test_run (libglib-2.0.so.0+0x7e055) #21 main ../tests/unit/test-qht.c:259 (test-qht+0xd2c6) #22 __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 (libc.so.6+0x29d8f) #23 __libc_start_main_impl ../csu/libc-start.c:392 (libc.so.6+0x29e3f) #24 _start (test-qht+0xdb44) Signed-off-by: Emilio Cota --- util/qht.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 93 insertions(+), 14 deletions(-) diff --git a/util/qht.c b/util/qht.c index 15866299e6..6174533f10 100644 --- a/util/qht.c +++ b/util/qht.c @@ -151,6 +151,22 @@ struct qht_bucket { QEMU_BUILD_BUG_ON(sizeof(struct qht_bucket) > QHT_BUCKET_ALIGN); +/* + * Under TSAN, we use striped locks instead of one lock per bucket chain. + * This avoids crashing under TSAN, since TSAN aborts the program if more than + * 64 locks are held (this is a hardcoded limit in TSAN). + * When resizing a QHT we grab all the buckets' locks, which can easily + * go over TSAN's limit. By using striped locks, we avoid this problem. + * + * Note: this number must be a power of two for easy index computation. + */ +#define QHT_TSAN_BUCKET_LOCKS_BITS 4 +#define QHT_TSAN_BUCKET_LOCKS (1 << QHT_TSAN_BUCKET_LOCKS_BITS) + +struct qht_tsan_lock { + QemuSpin lock; +} QEMU_ALIGNED(QHT_BUCKET_ALIGN); + /** * struct qht_map - structure to track an array of buckets * @rcu: used by RCU. Keep it as the top field in the struct to help valgrind @@ -160,6 +176,7 @@ QEMU_BUILD_BUG_ON(sizeof(struct qht_bucket) > QHT_BUCKET_ALIGN); * @n_added_buckets: number of added (i.e. "non-head") buckets * @n_added_buckets_threshold: threshold to trigger an upward resize once the * number of added buckets surpasses it. + * @tsan_bucket_locks: Array of striped locks to be used only under TSAN. * * Buckets are tracked in what we call a "map", i.e. this structure. */ @@ -169,6 +186,9 @@ struct qht_map { size_t n_buckets; size_t n_added_buckets; size_t n_added_buckets_threshold; +#ifdef CONFIG_TSAN + struct qht_tsan_lock tsan_bucket_locks[QHT_TSAN_BUCKET_LOCKS]; +#endif }; /* trigger a resize when n_added_buckets > n_buckets / div */ @@ -229,10 +249,68 @@ static inline size_t qht_elems_to_buckets(size_t n_elems) return pow2ceil(n_elems / QHT_BUCKET_ENTRIES); } -static inline void qht_head_init(struct qht_bucket *b) +/* + * When using striped locks (i.e. under TSAN), we have to be careful not + * to operate on the same lock twice (e.g. when iterating through all buckets). + * We achieve this by operating only on each stripe's first matching lock. + */ +static inline void qht_do_if_first_in_stripe(const struct qht_map *map, + struct qht_bucket *b, + void (*func)(QemuSpin *spin)) +{ +#ifdef CONFIG_TSAN + unsigned long bucket_idx = b - map->buckets; + bool is_first_in_stripe = (bucket_idx >> QHT_TSAN_BUCKET_LOCKS_BITS) == 0; + if (is_first_in_stripe) { + unsigned long lock_idx = bucket_idx & (QHT_TSAN_BUCKET_LOCKS - 1); + func(&map->tsan_bucket_locks[lock_idx]); + } +#else + func(&b->lock); +#endif +} + +static inline void qht_bucket_lock_init(const struct qht_map *map, + struct qht_bucket *b) +{ + qht_do_if_first_in_stripe(map, b, qemu_spin_init); +} + +static inline void qht_bucket_lock_destroy(const struct qht_map *map, + struct qht_bucket *b) +{ + qht_do_if_first_in_stripe(map, b, (void (*)(QemuSpin *))qemu_spin_destroy); +} + +static inline void qht_bucket_lock_do(const struct qht_map *map, + struct qht_bucket *b, + void (*func)(QemuSpin *lock)) +{ +#ifdef CONFIG_TSAN + unsigned long bucket_idx = b - map->buckets; + unsigned long lock_idx = bucket_idx & (QHT_TSAN_BUCKET_LOCKS - 1); + func(&map->tsan_bucket_locks[lock_idx]); +#else + func(&b->lock); +#endif +} + +static inline void qht_bucket_lock(const struct qht_map *map, + struct qht_bucket *b) +{ + qht_bucket_lock_do(map, b, qemu_spin_lock); +} + +static inline void qht_bucket_unlock(const struct qht_map *map, + struct qht_bucket *b) +{ + qht_bucket_lock_do(map, b, qemu_spin_unlock); +} + +static inline void qht_head_init(struct qht_map *map, struct qht_bucket *b) { memset(b, 0, sizeof(*b)); - qemu_spin_init(&b->lock); + qht_bucket_lock_init(map, b); seqlock_init(&b->sequence); } @@ -250,7 +328,7 @@ static void qht_map_lock_buckets(struct qht_map *map) for (i = 0; i < map->n_buckets; i++) { struct qht_bucket *b = &map->buckets[i]; - qemu_spin_lock(&b->lock); + qht_do_if_first_in_stripe(map, b, qemu_spin_lock); } } @@ -261,7 +339,7 @@ static void qht_map_unlock_buckets(struct qht_map *map) for (i = 0; i < map->n_buckets; i++) { struct qht_bucket *b = &map->buckets[i]; - qemu_spin_unlock(&b->lock); + qht_do_if_first_in_stripe(map, b, qemu_spin_unlock); } } @@ -308,7 +386,7 @@ void qht_map_lock_buckets__no_stale(struct qht *ht, struct qht_map **pmap) * Get a head bucket and lock it, making sure its parent map is not stale. * @pmap is filled with a pointer to the bucket's parent map. * - * Unlock with qemu_spin_unlock(&b->lock). + * Unlock with qht_bucket_unlock. * * Note: callers cannot have ht->lock held. */ @@ -322,18 +400,18 @@ struct qht_bucket *qht_bucket_lock__no_stale(struct qht *ht, uint32_t hash, map = qatomic_rcu_read(&ht->map); b = qht_map_to_bucket(map, hash); - qemu_spin_lock(&b->lock); + qht_bucket_lock(map, b); if (likely(!qht_map_is_stale__locked(ht, map))) { *pmap = map; return b; } - qemu_spin_unlock(&b->lock); + qht_bucket_unlock(map, b); /* we raced with a resize; acquire ht->lock to see the updated ht->map */ qht_lock(ht); map = ht->map; b = qht_map_to_bucket(map, hash); - qemu_spin_lock(&b->lock); + qht_bucket_lock(map, b); qht_unlock(ht); *pmap = map; return b; @@ -345,12 +423,13 @@ static inline bool qht_map_needs_resize(const struct qht_map *map) map->n_added_buckets_threshold; } -static inline void qht_chain_destroy(const struct qht_bucket *head) +static inline void qht_chain_destroy(const struct qht_map *map, + struct qht_bucket *head) { struct qht_bucket *curr = head->next; struct qht_bucket *prev; - qemu_spin_destroy(&head->lock); + qht_bucket_lock_destroy(map, head); while (curr) { prev = curr; curr = curr->next; @@ -364,7 +443,7 @@ static void qht_map_destroy(struct qht_map *map) size_t i; for (i = 0; i < map->n_buckets; i++) { - qht_chain_destroy(&map->buckets[i]); + qht_chain_destroy(map, &map->buckets[i]); } qemu_vfree(map->buckets); g_free(map); @@ -390,7 +469,7 @@ static struct qht_map *qht_map_create(size_t n_buckets) map->buckets = qemu_memalign(QHT_BUCKET_ALIGN, sizeof(*map->buckets) * n_buckets); for (i = 0; i < n_buckets; i++) { - qht_head_init(&map->buckets[i]); + qht_head_init(map, &map->buckets[i]); } return map; } @@ -638,7 +717,7 @@ bool qht_insert(struct qht *ht, void *p, uint32_t hash, void **existing) b = qht_bucket_lock__no_stale(ht, hash, &map); prev = qht_insert__locked(ht, map, b, p, hash, &needs_resize); qht_bucket_debug__locked(b); - qemu_spin_unlock(&b->lock); + qht_bucket_unlock(map, b); if (unlikely(needs_resize) && ht->mode & QHT_MODE_AUTO_RESIZE) { qht_grow_maybe(ht); @@ -749,7 +828,7 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t hash) b = qht_bucket_lock__no_stale(ht, hash, &map); ret = qht_remove__locked(b, p, hash); qht_bucket_debug__locked(b); - qemu_spin_unlock(&b->lock); + qht_bucket_unlock(map, b); return ret; } From patchwork Sun Jan 8 16:39:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 13092582 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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5BB31C53210 for ; Sun, 8 Jan 2023 16:40:06 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pEYi6-0001Bi-Ph; Sun, 08 Jan 2023 11:39:50 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pEYi5-0001BF-BX for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:49 -0500 Received: from mail-qt1-x831.google.com ([2607:f8b0:4864:20::831]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pEYi3-0002gl-Tl for qemu-devel@nongnu.org; Sun, 08 Jan 2023 11:39:49 -0500 Received: by mail-qt1-x831.google.com with SMTP id j15so672297qtv.4 for ; Sun, 08 Jan 2023 08:39:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HmY5q5T4tQyiUixjlDijQBP6aFMcfJLYqoAw6OnSdDI=; b=dnsmqHIkrz253qbjYEyxj583ixSyyDa6fFHqTgHzX1Bi3jWVdjnFNLoJe1N96Hu5G6 lVKz2UVkv2m24zzHMHPFxaMyYSYo3TqDcIAnPq+42ISpxlad2KGRmXJhwcI56IPGbg+H b79pBvMy4vqGo32EyGFD7cvlAO8i/eiR+4GleeTST6BMJloUrURAka6+vRImL8XsUiXl htYLDo3CWSS6Deyz1ioQdiSh/NzmzAwrUXfELGoxKxkiJM3Tt3QQQ6bKThpYE8sVka5P o0Dv8dx/qfhLUMFRdm7A1OVKszIDG2CMF72IzgmTC/bVDokJY1nmkk/gcC1l8UcbZ79n qqnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HmY5q5T4tQyiUixjlDijQBP6aFMcfJLYqoAw6OnSdDI=; b=vyeD0pykqvYLtBWC78pFwHMLD3l8wbiHAVzOWccr+kiiNJaon4RmvNhNEQ8rpmRiSx HHfpNTz6+KY/Is3ZpeU/TSxox2tuqErVckSyVvh+5DjAdRc4z+h3o3b2m2JbAubp5TRs O6WBdBLzjed96bonjcsQMRfmgrZ2GAlW2UwTgx+yu7J+K479IGhQJx+yJ/D1kG0uJyKH NlQqHwOlsWLJ93zHBbiqQs/ibslJEo2jmSvNG7NqtkoiDJ3esJ759y9nXuC2XMkYOfQ5 u79c4L+prkmDWWGC144OLGJWSLGCWsi0OfG+fKl7Tqm2CtPFGAtsEQc/G8Dr8UXjDlIV 9Flg== X-Gm-Message-State: AFqh2kqQ3xVLch7g3hVevPEDsv/emWRSudN9uiVz0SsUs402V1QgEjBj d9jMupScC5Z0mS8h9ztuFEyhm6eLLJ/JCSQUYJ4vRw== X-Google-Smtp-Source: AMrXdXuKHXNbcNvj85VKiPeRc33mZE3G+vpjl4aYXnM63N8VvUdjqAHLCNYf6WoMmcc8i4/MemXywQ== X-Received: by 2002:ac8:7452:0:b0:3a5:e9fc:aa87 with SMTP id h18-20020ac87452000000b003a5e9fcaa87mr87336569qtr.59.1673195986792; Sun, 08 Jan 2023 08:39:46 -0800 (PST) Received: from localhost ([185.156.46.146]) by smtp.gmail.com with ESMTPSA id a22-20020a05620a103600b006fbaf9c1b70sm3864759qkk.133.2023.01.08.08.39.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 08 Jan 2023 08:39:46 -0800 (PST) From: Emilio Cota To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , Paolo Bonzini , Richard Henderson , Emilio Cota Subject: [PATCH 4/4] plugins: make qemu_plugin_user_exit's locking order consistent with fork_start's Date: Sun, 8 Jan 2023 11:39:05 -0500 Message-Id: <20230108163905.59601-5-cota@braap.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230108163905.59601-1-cota@braap.org> References: <20230108163905.59601-1-cota@braap.org> MIME-Version: 1.0 Received-SPF: softfail client-ip=2607:f8b0:4864:20::831; envelope-from=cota@braap.org; helo=mail-qt1-x831.google.com X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org To fix potential deadlocks as reported by tsan. Signed-off-by: Emilio Cota Reviewed-by: Richard Henderson --- plugins/core.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/plugins/core.c b/plugins/core.c index ccb770a485..728bacef95 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -500,10 +500,17 @@ void qemu_plugin_user_exit(void) enum qemu_plugin_event ev; CPUState *cpu; - QEMU_LOCK_GUARD(&plugin.lock); - + /* + * Locking order: we must acquire locks in an order that is consistent + * with the one in fork_start(). That is: + * - start_exclusive(), which acquires qemu_cpu_list_lock, + * must be called before acquiring plugin.lock. + * - tb_flush(), which acquires mmap_lock(), must be called + * while plugin.lock is not held. + */ start_exclusive(); + qemu_rec_mutex_lock(&plugin.lock); /* un-register all callbacks except the final AT_EXIT one */ for (ev = 0; ev < QEMU_PLUGIN_EV_MAX; ev++) { if (ev != QEMU_PLUGIN_EV_ATEXIT) { @@ -513,13 +520,12 @@ void qemu_plugin_user_exit(void) } } } - - tb_flush(current_cpu); - CPU_FOREACH(cpu) { qemu_plugin_disable_mem_helpers(cpu); } + qemu_rec_mutex_unlock(&plugin.lock); + tb_flush(current_cpu); end_exclusive(); /* now it's safe to handle the exit case */