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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 178B9C5517A for ; Fri, 23 Oct 2020 07:14:30 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 878312168B for ; Fri, 23 Oct 2020 07:14:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=rasmusvillemoes.dk header.i=@rasmusvillemoes.dk header.b="fIw2nnxw" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 878312168B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=rasmusvillemoes.dk Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A889E6B005D; Fri, 23 Oct 2020 03:14:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A11016B0062; Fri, 23 Oct 2020 03:14:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D7496B0068; Fri, 23 Oct 2020 03:14:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0029.hostedemail.com [216.40.44.29]) by kanga.kvack.org (Postfix) with ESMTP id 57E566B005D for ; Fri, 23 Oct 2020 03:14:27 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id F13A9180AD817 for ; Fri, 23 Oct 2020 07:14:26 +0000 (UTC) X-FDA: 77402326932.15.man90_460d59627257 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id CAEB11814B0C1 for ; Fri, 23 Oct 2020 07:14:26 +0000 (UTC) X-HE-Tag: man90_460d59627257 X-Filterd-Recvd-Size: 11342 Received: from mail-ej1-f66.google.com (mail-ej1-f66.google.com [209.85.218.66]) by imf49.hostedemail.com (Postfix) with ESMTP for ; Fri, 23 Oct 2020 07:14:26 +0000 (UTC) Received: by mail-ej1-f66.google.com with SMTP id qh17so942628ejb.6 for ; Fri, 23 Oct 2020 00:14:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rasmusvillemoes.dk; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=tV2JdgeetWkMo3b0CNfPI5PIf7h/qWNZxkMeAZIAXgY=; b=fIw2nnxwmkbu7DraOrhc1cAb6DP8f6lxZtkBYUHubh5it1qNClYsZpnGZ3YLDsL/zb Za90R2NRP5hBvIBd5J3NkV9nsLXUVzQSxDVKiLjnTELEbQSjkvAuDYIOFGxjsOYXTjpo KbgJvqauR7ASZGWckCkRxGOSU/6znYiPGiN7w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=tV2JdgeetWkMo3b0CNfPI5PIf7h/qWNZxkMeAZIAXgY=; b=PFqIBZQMbxir7eIAf/ydouy0Z0qhiURuJ5aL9fMBGxRQWsIwEpsHq19cGQI5ZpCABf /iwmJMa+6czHQqPtSzCOpoCffMAOE3CBQw/xIMYND/a71CU7UOFgWsmPg7OiURbAia+6 K5s3zVof+9NHFoNEcdSnYpUHo4uEubAcm9Ul/GitRzZ/wUgQhgUfAHfjNnSnX7DvQW3F K0N8P6kW7ZlNfWv3cT3WPekt49eCZ4d5/NozcEG876mZYy2TTi/CR3pjishjsU85sKwK XhjDn13EUVPP3sTaln/GfU37ULFJeuS5rcLhHpGBqzn+TXeLm3spewaOAvXpT9LMIdxQ j/Tg== X-Gm-Message-State: AOAM532hkXLIzc9lknU0epeOGn8+HU8vc7usnyee8xImSmo3eH+D2dGw 9Y7TM4a0IXSn1iEgteIPYM8EnQ== X-Google-Smtp-Source: ABdhPJy047+CmhTalOZN6C5t0TMaQd7AP4ay6FCicu99E7Q9zHQUqdZgSji/CmQEuleg6vEhWhL9+Q== X-Received: by 2002:a17:906:a1d4:: with SMTP id bx20mr664255ejb.262.1603437264294; Fri, 23 Oct 2020 00:14:24 -0700 (PDT) Received: from [172.16.11.132] ([81.216.59.226]) by smtp.gmail.com with ESMTPSA id h17sm287717ejf.98.2020.10.23.00.14.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 23 Oct 2020 00:14:23 -0700 (PDT) Subject: Re: [LTP] mmstress[1309]: segfault at 7f3d71a36ee8 ip 00007f3d77132bdf sp 00007f3d71a36ee8 error 4 in libc-2.27.so[7f3d77058000+1aa000] To: Sean Christopherson , Linus Torvalds Cc: =?UTF-8?Q?Daniel_D=c3=adaz?= , Naresh Kamboju , Stephen Rothwell , "Matthew Wilcox (Oracle)" , zenglg.jy@cn.fujitsu.com, "Peter Zijlstra (Intel)" , Viresh Kumar , X86 ML , open list , lkft-triage@lists.linaro.org, "Eric W. Biederman" , linux-mm , linux-m68k , Linux-Next Mailing List , Thomas Gleixner , kasan-dev , Dmitry Vyukov , Geert Uytterhoeven , Christian Brauner , Ingo Molnar , LTP List , Al Viro References: <20201023050214.GG23681@linux.intel.com> From: Rasmus Villemoes Message-ID: <356811ab-cb08-7685-ca01-fe58b5654953@rasmusvillemoes.dk> Date: Fri, 23 Oct 2020 09:14:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201023050214.GG23681@linux.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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 23/10/2020 07.02, Sean Christopherson wrote: > On Thu, Oct 22, 2020 at 08:05:05PM -0700, Linus Torvalds wrote: >> On Thu, Oct 22, 2020 at 6:36 PM Daniel D=EDaz = wrote: >>> >>> The kernel Naresh originally referred to is here: >>> https://builds.tuxbuild.com/SCI7Xyjb7V2NbfQ2lbKBZw/ >> >> Thanks. >> >> And when I started looking at it, I realized that my original idea >> ("just look for __put_user_nocheck_X calls, there aren't so many of >> those") was garbage, and that I was just being stupid. >> >> Yes, the commit that broke was about __put_user(), but in order to not >> duplicate all the code, it re-used the regular put_user() >> infrastructure, and so all the normal put_user() calls are potential >> problem spots too if this is about the compiler interaction with KASAN >> and the asm changes. >> >> So it's not just a couple of special cases to look at, it's all the >> normal cases too. >> >> Ok, back to the drawing board, but I think reverting it is probably >> the right thing to do if I can't think of something smart. >> >> That said, since you see this on x86-64, where the whole ugly trick wi= th that >> >> register asm("%"_ASM_AX) >> >> is unnecessary (because the 8-byte case is still just a single >> register, no %eax:%edx games needed), it would be interesting to hear >> if the attached patch fixes it. That would confirm that the problem >> really is due to some register allocation issue interaction (or, >> alternatively, it would tell me that there's something else going on). >=20 > I haven't reproduced the crash, but I did find a smoking gun that confi= rms the > "register shenanigans are evil shenanigans" theory. I ran into a simil= ar thing > recently where a seemingly innocuous line of code after loading a value= into a > register variable wreaked havoc because it clobbered the input register= . >=20 > This put_user() in schedule_tail(): >=20 > if (current->set_child_tid) > put_user(task_pid_vnr(current), current->set_child_tid); >=20 > generates the following assembly with KASAN out-of-line: >=20 > 0xffffffff810dccc9 <+73>: xor %edx,%edx > 0xffffffff810dcccb <+75>: xor %esi,%esi > 0xffffffff810dcccd <+77>: mov %rbp,%rdi > 0xffffffff810dccd0 <+80>: callq 0xffffffff810bf5e0 <__task_pid_nr_n= s> > 0xffffffff810dccd5 <+85>: mov %r12,%rdi > 0xffffffff810dccd8 <+88>: callq 0xffffffff81388c60 <__asan_load8> > 0xffffffff810dccdd <+93>: mov 0x590(%rbp),%rcx > 0xffffffff810dcce4 <+100>: callq 0xffffffff817708a0 <__put_user_4> > 0xffffffff810dcce9 <+105>: pop %rbx > 0xffffffff810dccea <+106>: pop %rbp > 0xffffffff810dcceb <+107>: pop %r12 >=20 > __task_pid_nr_ns() returns the pid in %rax, which gets clobbered by > __asan_load8()'s check on current for the current->set_child_tid derefe= rence. >=20 Yup, and you don't need KASAN to implicitly generate function calls for you. With x86_64 defconfig, I get extern u64 __user *get_destination(int x, int y); void pu_test(void) { u64 big =3D 0x1234abcd5678; if (put_user(big, get_destination(4, 5))) pr_warn("uh"); } to generate 0000000000004d60 : 4d60: 53 push %rbx 4d61: be 05 00 00 00 mov $0x5,%esi 4d66: bf 04 00 00 00 mov $0x4,%edi 4d6b: e8 00 00 00 00 callq 4d70 4d6c: R_X86_64_PC32 get_destination-0x4 4d70: 48 89 c1 mov %rax,%rcx 4d73: e8 00 00 00 00 callq 4d78 4d74: R_X86_64_PC32 __put_user_8-0x4 4d78: 85 c9 test %ecx,%ecx 4d7a: 75 02 jne 4d7e 4d7c: 5b pop %rbx 4d7d: c3 retq 4d7e: 5b pop %rbx 4d7f: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 4d82: R_X86_64_32S .rodata.str1.1+0xfa 4d86: e9 00 00 00 00 jmpq 4d8b 4d87: R_X86_64_PC32 printk-0x4 That's certainly garbage. Now, I don't know if it's a sufficient fix (or could break something else), but the obvious first step of rearranging so that the ptr argument is evaluated before the assignment to __val_pu diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uacces= s.h index 477c503f2753..b5d3290fcd09 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -235,13 +235,13 @@ extern void __put_user_nocheck_8(void); #define do_put_user_call(fn,x,ptr) \ ({ \ int __ret_pu; \ - register __typeof__(*(ptr)) __val_pu asm("%"_ASM_AX); \ + __typeof__(ptr) __ptr =3D (ptr); = \ + register __typeof__(*(ptr)) __val_pu asm("%"_ASM_AX) =3D (x); = \ __chk_user_ptr(ptr); \ - __val_pu =3D (x); = \ asm volatile("call __" #fn "_%P[size]" \ : "=3Dc" (__ret_pu), = \ ASM_CALL_CONSTRAINT \ - : "0" (ptr), \ + : "0" (__ptr), \ "r" (__val_pu), \ [size] "i" (sizeof(*(ptr))) \ :"ebx"); \ at least gets us 0000000000004d60 : 4d60: 53 push %rbx 4d61: be 05 00 00 00 mov $0x5,%esi 4d66: bf 04 00 00 00 mov $0x4,%edi 4d6b: e8 00 00 00 00 callq 4d70 4d6c: R_X86_64_PC32 get_destination-0x4 4d70: 48 89 c1 mov %rax,%rcx 4d73: 48 b8 78 56 cd ab 34 movabs $0x1234abcd5678,%rax 4d7a: 12 00 00 4d7d: e8 00 00 00 00 callq 4d82 4d7e: R_X86_64_PC32 __put_user_8-0x4 FWIW, https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html does warn about function calls and other things that might clobber the register variables between the assignment and the use as an input (though the case of evaluating other input operands is not explicitly mentioned). Rasmus