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 DCE86C433EF for ; Thu, 12 May 2022 12:25:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 757F08D0001; Thu, 12 May 2022 08:25:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 70D216B0078; Thu, 12 May 2022 08:25:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 55A948D0001; Thu, 12 May 2022 08:25:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 430A06B0075 for ; Thu, 12 May 2022 08:25:07 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 23F3D12052E for ; Thu, 12 May 2022 12:25:07 +0000 (UTC) X-FDA: 79457010654.20.BDD27CC Received: from mail-yb1-f180.google.com (mail-yb1-f180.google.com [209.85.219.180]) by imf08.hostedemail.com (Postfix) with ESMTP id 50F571600A2 for ; Thu, 12 May 2022 12:24:52 +0000 (UTC) Received: by mail-yb1-f180.google.com with SMTP id m190so9424050ybf.4 for ; Thu, 12 May 2022 05:25:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=D4KhC5f7akNZCYIX8CtlWMuJDmlYSG1ZIcg9H4X+0cs=; b=sXaJnq0WTlzePRDnCEPZnv1ywN9iZDx3YdJpMPUTYdKyBc8fpCzfJxYzshSq0xZyPe 3qjM6nIbUB2rt1rmwfWS/Jcfqq50vfkOvDQfANthsZF9YuSm9eHP0oDAoLD+L4fPR+El nBRGApfUqlwsgLqYtQyQjb17FCffj5UWbAxAZXKzfK3Vq9PkBEHfYg3N77c4XzqbJpv9 x/FsGiWdRSgBll1tVMNM8DbxLdSHE/ITKEww13LBKWvcDPxGnKdO7EtCXTdbFAVZrIgg oehXpKop+Vl5sNUZ1rkni1wpkj2K+Z+snfW+rRiZ55fAaAqe+wK9Avr49louivL9fuEw 3l3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=D4KhC5f7akNZCYIX8CtlWMuJDmlYSG1ZIcg9H4X+0cs=; b=3gh/VnuPOrl4I595serXb+ijOeWhZtUG4A54WxnQhzkuk5cXwFg/q1S5uYK9wtvks0 8sBcpZcr5PmDSpbUSYg1lxEQ73DQgus2jGg6zP5i+lwTG4Wq4WV54rwwPg5WULlP67gf qfmMUWhUG399Ovpe4WQCDEb33kAIcBEQWWcDV3g/DwaiLqMoS8SpAY5ucNyq1Mi9BBcf e6BNQVQmPswAbA2T4Po7UEHTWc1hv5UvR+bniT1VyH001fsqK9Tv78xqoofuQpMF4Guo gFRPQGfAErv64yPwB7QNBZHVbp63siRZKhcYAXt9QqPOpPE+MHs17O1ZftZSVIuTr+/n 3eWw== X-Gm-Message-State: AOAM532o4JWveac3QC+1Gs82nxPgA8pS6FQjSg5nCk6LoXCybf/buB4Q 3Q7EjW8KlTPdTxXkHAFh8t/B7i6mgppopXQ8+EE5Ag== X-Google-Smtp-Source: ABdhPJyoD+H2LGgZwY+Bi30AMPIZBme8GNjgmAInKiNdqlZrW6PHBOWbalHNrR1jrF+lMZFQr9DfRd3NZTkSSxTJSUs= X-Received: by 2002:a25:aa62:0:b0:648:590f:5a53 with SMTP id s89-20020a25aa62000000b00648590f5a53mr29319927ybi.5.1652358305516; Thu, 12 May 2022 05:25:05 -0700 (PDT) MIME-Version: 1.0 References: <20220426164315.625149-1-glider@google.com> <20220426164315.625149-29-glider@google.com> <87a6c6y7mg.ffs@tglx> <87y1zjlhmj.ffs@tglx> <878rrfiqyr.ffs@tglx> <87k0ayhc43.ffs@tglx> <87h762h5c2.ffs@tglx> <871qx2r09k.ffs@tglx> In-Reply-To: <871qx2r09k.ffs@tglx> From: Alexander Potapenko Date: Thu, 12 May 2022 14:24:29 +0200 Message-ID: Subject: Re: [PATCH v3 28/46] kmsan: entry: handle register passing from uninstrumented code To: Thomas Gleixner Cc: Alexander Viro , Andrew Morton , Andrey Konovalov , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Christoph Hellwig , Christoph Lameter , David Rientjes , Dmitry Vyukov , Eric Dumazet , Greg Kroah-Hartman , Herbert Xu , Ilya Leoshkevich , Ingo Molnar , Jens Axboe , Joonsoo Kim , Kees Cook , Marco Elver , Mark Rutland , Matthew Wilcox , "Michael S. Tsirkin" , Pekka Enberg , Peter Zijlstra , Petr Mladek , Steven Rostedt , Vasily Gorbik , Vegard Nossum , Vlastimil Babka , kasan-dev , Linux Memory Management List , Linux-Arch , LKML Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 50F571600A2 X-Stat-Signature: fyc8pmk3s79x5m7rqb45br7f8dd5stsn Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=sXaJnq0W; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of glider@google.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=glider@google.com X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1652358292-712514 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: On Mon, May 9, 2022 at 9:09 PM Thomas Gleixner wrote: > > On Mon, May 09 2022 at 18:50, Alexander Potapenko wrote: > > Indeed, calling kmsan_unpoison_memory() in irqentry_enter() was > > supposed to be enough, but we have code in kmsan_unpoison_memory() (as > > well as other runtime functions) that checks for kmsan_in_runtime() > > and bails out to prevent potential recursion if KMSAN code starts > > calling itself. > > > > kmsan_in_runtime() is implemented as follows: > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > static __always_inline bool kmsan_in_runtime(void) > > { > > if ((hardirq_count() >> HARDIRQ_SHIFT) > 1) > > return true; > > return kmsan_get_context()->kmsan_in_runtime; > > } > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > (see the code here: > > https://lore.kernel.org/lkml/20220426164315.625149-13-glider@google.com= /#Z31mm:kmsan:kmsan.h) > > > > If we are running in the task context (in_task()=3D=3Dtrue), > > kmsan_get_context() returns a per-task `struct *kmsan_ctx`. > > If `in_task()=3D=3Dfalse` and `hardirq_count()>>HARDIRQ_SHIFT=3D=3D1`, = it > > returns a per-CPU one. > > Otherwise kmsan_in_runtime() is considered true to avoid dealing with > > nested interrupts. > > > > So in the case when `hardirq_count()>>HARDIRQ_SHIFT` is greater than > > 1, kmsan_in_runtime() becomes a no-op, which leads to false positives. > > But, that'd only > 1 when there is a nested interrupt, which is not the > case. Interrupt handlers keep interrupts disabled. The last exception fro= m > that rule was some legacy IDE driver which is gone by now. That's good to know, then we probably don't need this hardirq_count() check anymore. > So no, not a good explanation either. After looking deeper I see that unpoisoning was indeed skipped because kmsan_in_runtime() returned true, but I was wrong about the root cause. The problem was not caused by a nested hardirq, but rather by the fact that the KMSAN hook in irqentry_enter() was called with in_task()=3D=3D1. Roughly said, T0 was running some code in the task context, then it started executing KMSAN instrumentation and entered the runtime by setting current->kmsan_ctx.kmsan_in_runtime. Then an IRQ kicked in and started calling asm_sysvec_apic_timer_interrupt() =3D> sysvec_apic_timer_interrupt(regs) =3D> irqentry_enter(regs) - but at that point in_task() was still true, therefore kmsan_unpoison_memory() became a no-op. As far as I can see, it is irq_enter_rcu() that makes in_task() return 0 by incrementing the preempt count in __irq_enter_raw(), so our unpoisoning can only work if we perform it after we enter the IRQ context. I think the best that can be done here is (as suggested above) to provide some kmsan_unpoison_pt_regs() function that will only be called from the entry points and won't be doing reentrancy checks. It should be safe, because unpoisoning boils down to calculating shadow/origin addresses and calling memset() on them, no instrumented code will be involved. We could try to figure out the places in idtentry code where normal kmsan_unpoison_memory() can be called in IRQ context, but as far as I can see it will depend on the type of the entry point. Another way to deal with the problem is to not rely on in_task(), but rather use some per-cpu counter in irqentry_enter()/irqentry_exit() to figure out whether we are in IRQ code already. However this is only possible irqentry_enter() itself guarantees that the execution cannot be rescheduled to another CPU - is that the case? > Thanks, > > tglx > > -- > You received this message because you are subscribed to the Google Groups= "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an= email to kasan-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgi= d/kasan-dev/871qx2r09k.ffs%40tglx. --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Falls Sie diese f=C3=A4lschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, l=C3=B6schen Sie alle Kopien und Anh=C3=A4nge davon und lassen Sie = mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde. This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.