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 D3D18C433F5 for ; Fri, 6 May 2022 18:41:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4231E6B0071; Fri, 6 May 2022 14:41:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3D3306B0073; Fri, 6 May 2022 14:41:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2762A6B0074; Fri, 6 May 2022 14:41:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0235.hostedemail.com [216.40.44.235]) by kanga.kvack.org (Postfix) with ESMTP id 187B76B0071 for ; Fri, 6 May 2022 14:41:06 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id C7A61182890DB for ; Fri, 6 May 2022 18:41:05 +0000 (UTC) X-FDA: 79436185290.30.436D752 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf27.hostedemail.com (Postfix) with ESMTP id E570040086 for ; Fri, 6 May 2022 18:41:02 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1651862462; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=dbruHfD7L5ui13yo40XACqjP4QTu0Yh5HyGtytqH9h0=; b=toC9h/bBwe9VN8u0hUjpXZdGLr+EKTd0NwPxoibysZHnQ+TBBXS2E0bhBMWj05jHILbNUK SvdtClNM77RiRFJ/LNGfReUgCpisxqP/hFYB+n+XxiUxV4WruMJEdzaAqLvW5MrL/ZhyCX If7RuFXFoPo3d0+qqtXuxRp7HKE+jCnpzh060a8MhUqh5WlTx73Na0uCiJG5IQ6bmgzVKr 3io8ZEPWGWuAUxQB6j7nvde2mxp3AGn/uAP8x2DLvWSJBGR8itlEAg2KPbBFtHX0Wc0lpB fNaTb6o1nji7JWcAHXB9VmdptlPDMjp8g3bU377UrR3Lbjczd9mO3gzfIXyjRg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1651862462; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=dbruHfD7L5ui13yo40XACqjP4QTu0Yh5HyGtytqH9h0=; b=8YaG10zQ5qm1FsH6PnUal8N/UCNILrym23WsmzAwEeH3cDqPKu9wU9s+9+YmXF8OHFEOcR it8ZOdn6PPQ+ZRDA== To: Alexander Potapenko 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 Subject: Re: [PATCH v3 28/46] kmsan: entry: handle register passing from uninstrumented code In-Reply-To: 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> Date: Fri, 06 May 2022 20:41:01 +0200 Message-ID: <87h762h5c2.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Queue-Id: E570040086 X-Stat-Signature: ce9acny8o45qr1ry36r58goyhazujo98 X-Rspam-User: Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b="toC9h/bB"; dkim=pass header.d=linutronix.de header.s=2020e header.b=8YaG10zQ; spf=pass (imf27.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de; dmarc=pass (policy=none) header.from=linutronix.de X-Rspamd-Server: rspam09 X-HE-Tag: 1651862462-378192 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 Fri, May 06 2022 at 19:41, Alexander Potapenko wrote: > On Fri, May 6, 2022 at 6:14 PM Thomas Gleixner wrote: >> sysvec_apic_timer_interrupt() invokes irqentry_enter() _before_ >> set_irq_regs() and irqentry_enter() unpoisons @reg. >> >> Confused... > > As far as I can tell in this case sysvect_apic_timer_interrupt() is > called by the following code in arch/x86/kernel/idt.c: > > INTG(LOCAL_TIMER_VECTOR, asm_sysvec_apic_timer_interrupt), > > , which does not use IDTENTRY_SYSVEC framework and thus does not call > irqentry_enter(). asm_sysvec_apic_timer_interrupt != sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c: DEFINE_IDTENTRY_SYSVEC(sysvec_apic_timer_interrupt) { .... #define DEFINE_IDTENTRY_SYSVEC(func) \ static void __##func(struct pt_regs *regs); \ \ __visible noinstr void func(struct pt_regs *regs) \ { \ irqentry_state_t state = irqentry_enter(regs); \ .... __##func (regs); \ .... } \ \ static noinline void __##func(struct pt_regs *regs) So it goes through that code path _before_ the actual implementation which does set_irq_regs() is reached. The callchain is: asm_sysvec_apic_timer_interrupt <- ASM entry in gate sysvec_apic_timer_interrupt(regs) <- noinstr C entry point irqentry_enter(regs) <- unpoisons @reg __sysvec_apic_timer_interrupt(regs) <- the actual handler set_irq_regs(regs) <- stores regs local_apic_timer_interrupt() ... tick_handler() <- One of the 4 variants regs = get_irq_regs(); <- retrieves regs update_process_times(user_tick = user_mode(regs)) account_process_tick(user_tick) irqtime_account_process_tick(user_tick) line 382: } else if { user_tick } <- KMSAN complains I'm even more confused now. > I guess handling those will require wrapping every interrupt gate into > a function that performs register unpoisoning? No, guessing does not help here. The gates point to the ASM entry point, which then invokes the C entry point. All C entry points use a DEFINE_IDTENTRY variant. Some of the DEFINE_IDTENTRY_* C entry points are not doing anything in the macro, but the C function either invokes irqentry_enter() or irqentry_nmi_enter() open coded _before_ invoking any instrumentable function. So the unpoisoning of @regs in these functions should tell KMSAN that @regs or something derived from @regs are not some random uninitialized values. There should be no difference between unpoisoning @regs in irqentry_enter() or in set_irq_regs(), right? If so, then the problem is definitely _not_ the idt entry code. > By the way, if it helps, I think we don't necessarily have to call > kmsan_unpoison_memory() from within the > instrumentation_begin()/instrumentation_end() region? > We could move the call to the beginning of irqentry_enter(), removing > unnecessary duplication. We could, but then you need to mark unpoison_memory() noinstr too and you have to add the unpoison into the syscall code. No win and irrelevant to the problem at hand. Thanks, tglx