From mboxrd@z Thu Jan 1 00:00:00 1970 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA4D9C433EF for ; Mon, 6 Dec 2021 05:05:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2988C6B007B; Mon, 6 Dec 2021 00:05:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 246B26B007D; Mon, 6 Dec 2021 00:05:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 10FC56B007E; Mon, 6 Dec 2021 00:05:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id F34076B007B for ; Mon, 6 Dec 2021 00:05:05 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id B670E181A870F for ; Mon, 6 Dec 2021 05:04:55 +0000 (UTC) X-FDA: 78886179750.13.3CA533E Received: from mail-il1-f175.google.com (mail-il1-f175.google.com [209.85.166.175]) by imf29.hostedemail.com (Postfix) with ESMTP id 6D6B0900013E for ; Mon, 6 Dec 2021 05:04:52 +0000 (UTC) Received: by mail-il1-f175.google.com with SMTP id j7so8984414ilk.13 for ; Sun, 05 Dec 2021 21:04:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tDhe7b+IhPRxxignxx1nDXp2K4ORmwOTZ8ivGctx3KA=; b=gDBAKWrWkjW69MD6bJRbbC4IAUISs6K3clEQYe9+qhYEM+3/ei2QA1p+eyWPl//PkZ MZs1sJjHKYkDAoANTwNTADXYKpdEjYIoi0R38SWvQThsyk9W7siLDhWHxaTtIEwvznxX KsHy0EM0d6MDcPv/1xG1+2t90T3HhVUCpqNxDVZnsHkUi1Y6eq+fBTeGiPCxYug4x2bg SVkrAd1qAIS0Ux/lvZ2x7r2K+56gFzhT1fwfVTxWtoVnmExYkIVzYe5aVUeiB2rCVfQ2 5yZPU9xKsAoMWF//IutAF8y9BC+AT6LIYHiIOXQZDA7ya17atidzw1OsV6Xm7smQnWZ3 PWFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tDhe7b+IhPRxxignxx1nDXp2K4ORmwOTZ8ivGctx3KA=; b=F16E7ZfkDFt3T8cbY6jIsbm9xtqCdcV6C20E480sCweeP8cRXmRS/ItSaEl7NGQhpx FLZtAgFsuODWomYTaX7gpBhDb4XH8ns782tHjBiq31OR8+HnA0s++kP6EKTI9XMU5Eni Vr6140m0XJwul45UW7XwXiTuqDfaN6KS7skYn9n5/ri0PW6mfO7DdAQj3AEZ5rZYRqOI gVgRwwPku20Np/6lCP2BzPiIPiBIm1uhWU4t3uEGx0x/DZT52y8D+ur+ACAZBNENOGRe nCjmdoafqqAdUYun3IuWGH6afYhMpeTiwQx0PdyJvs5kXV8tHtD2OWOblkLodU+IOshq Z5NQ== X-Gm-Message-State: AOAM530/AAz9zS+WvzxJVwLwJuVE/sySmnvyu1k1V4UYMN3sA6P8MzXf Zm/bwMqVpE7XwOfHdhaZrpk= X-Google-Smtp-Source: ABdhPJwaVNNy8KXHPX0diE+9BqZRmxfh/F+tlyo2HmuHIUQC0d5++KCQ+YdgK0rTeMwJsEVyI2Orbg== X-Received: by 2002:a05:6e02:1aa2:: with SMTP id l2mr31908443ilv.7.1638767094542; Sun, 05 Dec 2021 21:04:54 -0800 (PST) Received: from auth2-smtp.messagingengine.com (auth2-smtp.messagingengine.com. [66.111.4.228]) by smtp.gmail.com with ESMTPSA id c22sm5776377ioz.15.2021.12.05.21.04.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Dec 2021 21:04:53 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailauth.nyi.internal (Postfix) with ESMTP id 0841227C0054; Mon, 6 Dec 2021 00:04:51 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Mon, 06 Dec 2021 00:04:52 -0500 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrjedvgdejlecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesthdtredttddtvdenucfhrhhomhepuehoqhhunhcu hfgvnhhguceosghoqhhunhdrfhgvnhhgsehgmhgrihhlrdgtohhmqeenucggtffrrghtth gvrhhnpedvleeigedugfegveejhfejveeuveeiteejieekvdfgjeefudehfefhgfegvdeg jeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsoh hquhhnodhmvghsmhhtphgruhhthhhpvghrshhonhgrlhhithihqdeiledvgeehtdeigedq udejjeekheehhedvqdgsohhquhhnrdhfvghngheppehgmhgrihhlrdgtohhmsehfihigmh gvrdhnrghmvg X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 6 Dec 2021 00:04:50 -0500 (EST) Date: Mon, 6 Dec 2021 13:03:33 +0800 From: Boqun Feng To: Marco Elver Cc: "Paul E. McKenney" , Alexander Potapenko , Borislav Petkov , Dmitry Vyukov , Ingo Molnar , Mark Rutland , Peter Zijlstra , Thomas Gleixner , Waiman Long , Will Deacon , kasan-dev@googlegroups.com, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, llvm@lists.linux.dev, x86@kernel.org Subject: Re: [PATCH v3 08/25] kcsan: Show location access was reordered to Message-ID: References: <20211130114433.2580590-1-elver@google.com> <20211130114433.2580590-9-elver@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211130114433.2580590-9-elver@google.com> X-Stat-Signature: 1x8ykxwmi7ssycup1ct7nh3ntgjrre9g Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=gDBAKWrW; spf=pass (imf29.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.166.175 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 6D6B0900013E X-HE-Tag: 1638767092-311813 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi, On Tue, Nov 30, 2021 at 12:44:16PM +0100, Marco Elver wrote: > Also show the location the access was reordered to. An example report: > > | ================================================================== > | BUG: KCSAN: data-race in test_kernel_wrong_memorder / test_kernel_wrong_memorder > | > | read-write to 0xffffffffc01e61a8 of 8 bytes by task 2311 on cpu 5: > | test_kernel_wrong_memorder+0x57/0x90 > | access_thread+0x99/0xe0 > | kthread+0x2ba/0x2f0 > | ret_from_fork+0x22/0x30 > | > | read-write (reordered) to 0xffffffffc01e61a8 of 8 bytes by task 2310 on cpu 7: > | test_kernel_wrong_memorder+0x57/0x90 > | access_thread+0x99/0xe0 > | kthread+0x2ba/0x2f0 > | ret_from_fork+0x22/0x30 > | | > | +-> reordered to: test_kernel_wrong_memorder+0x80/0x90 > | Should this be "reordered from" instead of "reordered to"? For example, if the following case needs a smp_mb() between write to A and write to B, I think currently it will report as follow: foo() { WRITE_ONCE(A, 1); // let's say A's address is 0xaaaa bar() { WRITE_ONCE(B, 1); // Assume B's address is 0xbbbb // KCSAN find the problem here } } | write (reordered) to 0xaaaa of ...: | bar+0x... // address of the write to B | foo+0x... // address of the callsite to bar() | ... | | | +-> reordered to: foo+0x... // address of the write to A But since the access reported here is the write to A, so it's a "reordered from" instead of "reordered to"? Regards, Boqun > | Reported by Kernel Concurrency Sanitizer on: > | CPU: 7 PID: 2310 Comm: access_thread Not tainted 5.14.0-rc1+ #18 > | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014 > | ================================================================== > > Signed-off-by: Marco Elver > --- > kernel/kcsan/report.c | 35 +++++++++++++++++++++++------------ > 1 file changed, 23 insertions(+), 12 deletions(-) > > diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c > index 1b0e050bdf6a..67794404042a 100644 > --- a/kernel/kcsan/report.c > +++ b/kernel/kcsan/report.c > @@ -308,10 +308,12 @@ static int get_stack_skipnr(const unsigned long stack_entries[], int num_entries > > /* > * Skips to the first entry that matches the function of @ip, and then replaces > - * that entry with @ip, returning the entries to skip. > + * that entry with @ip, returning the entries to skip with @replaced containing > + * the replaced entry. > */ > static int > -replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned long ip) > +replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned long ip, > + unsigned long *replaced) > { > unsigned long symbolsize, offset; > unsigned long target_func; > @@ -330,6 +332,7 @@ replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned lon > func -= offset; > > if (func == target_func) { > + *replaced = stack_entries[skip]; > stack_entries[skip] = ip; > return skip; > } > @@ -342,9 +345,10 @@ replace_stack_entry(unsigned long stack_entries[], int num_entries, unsigned lon > } > > static int > -sanitize_stack_entries(unsigned long stack_entries[], int num_entries, unsigned long ip) > +sanitize_stack_entries(unsigned long stack_entries[], int num_entries, unsigned long ip, > + unsigned long *replaced) > { > - return ip ? replace_stack_entry(stack_entries, num_entries, ip) : > + return ip ? replace_stack_entry(stack_entries, num_entries, ip, replaced) : > get_stack_skipnr(stack_entries, num_entries); > } > > @@ -360,6 +364,14 @@ static int sym_strcmp(void *addr1, void *addr2) > return strncmp(buf1, buf2, sizeof(buf1)); > } > > +static void > +print_stack_trace(unsigned long stack_entries[], int num_entries, unsigned long reordered_to) > +{ > + stack_trace_print(stack_entries, num_entries, 0); > + if (reordered_to) > + pr_err(" |\n +-> reordered to: %pS\n", (void *)reordered_to); > +} > + > static void print_verbose_info(struct task_struct *task) > { > if (!task) > @@ -378,10 +390,12 @@ static void print_report(enum kcsan_value_change value_change, > struct other_info *other_info, > u64 old, u64 new, u64 mask) > { > + unsigned long reordered_to = 0; > unsigned long stack_entries[NUM_STACK_ENTRIES] = { 0 }; > int num_stack_entries = stack_trace_save(stack_entries, NUM_STACK_ENTRIES, 1); > - int skipnr = sanitize_stack_entries(stack_entries, num_stack_entries, ai->ip); > + int skipnr = sanitize_stack_entries(stack_entries, num_stack_entries, ai->ip, &reordered_to); > unsigned long this_frame = stack_entries[skipnr]; > + unsigned long other_reordered_to = 0; > unsigned long other_frame = 0; > int other_skipnr = 0; /* silence uninit warnings */ > > @@ -394,7 +408,7 @@ static void print_report(enum kcsan_value_change value_change, > if (other_info) { > other_skipnr = sanitize_stack_entries(other_info->stack_entries, > other_info->num_stack_entries, > - other_info->ai.ip); > + other_info->ai.ip, &other_reordered_to); > other_frame = other_info->stack_entries[other_skipnr]; > > /* @value_change is only known for the other thread */ > @@ -434,10 +448,9 @@ static void print_report(enum kcsan_value_change value_change, > other_info->ai.cpu_id); > > /* Print the other thread's stack trace. */ > - stack_trace_print(other_info->stack_entries + other_skipnr, > + print_stack_trace(other_info->stack_entries + other_skipnr, > other_info->num_stack_entries - other_skipnr, > - 0); > - > + other_reordered_to); > if (IS_ENABLED(CONFIG_KCSAN_VERBOSE)) > print_verbose_info(other_info->task); > > @@ -451,9 +464,7 @@ static void print_report(enum kcsan_value_change value_change, > get_thread_desc(ai->task_pid), ai->cpu_id); > } > /* Print stack trace of this thread. */ > - stack_trace_print(stack_entries + skipnr, num_stack_entries - skipnr, > - 0); > - > + print_stack_trace(stack_entries + skipnr, num_stack_entries - skipnr, reordered_to); > if (IS_ENABLED(CONFIG_KCSAN_VERBOSE)) > print_verbose_info(current); > > -- > 2.34.0.rc2.393.gf8c9666880-goog >