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 780B7C2BA18 for ; Thu, 20 Jun 2024 08:36:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0CB926B0165; Thu, 20 Jun 2024 04:36:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 07B6F6B0166; Thu, 20 Jun 2024 04:36:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E85646B016A; Thu, 20 Jun 2024 04:36:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id BEF486B0165 for ; Thu, 20 Jun 2024 04:36:53 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 63A021418BC for ; Thu, 20 Jun 2024 08:36:53 +0000 (UTC) X-FDA: 82250611506.26.2CE1C54 Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by imf05.hostedemail.com (Postfix) with ESMTP id 9BC99100010 for ; Thu, 20 Jun 2024 08:36:51 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="SORU/Lwz"; spf=pass (imf05.hostedemail.com: domain of glider@google.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=glider@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718872602; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/OUYcIAPu8Sr/lzFQuuddP6LocZmaVvpQ1N05tgsbXo=; b=J0BBy63nWU1ZK/HnLYE5iH6Y+xnx1yPFFLtt9O5heKvNGIbgyJho+hch25Rfv4Uxjjf0iA beBmlYf36KcFGbqh/VlSAMYc2ca56uzJrseeWR6iprmQGhSLO3CkV6cPuvC/nUQEOeLznG 50AFccTWfYPap30PUdk8VgNZGal1gzU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718872602; a=rsa-sha256; cv=none; b=PI/XuvXJhdwIq3oeQiyag7XvUTj9RMso3JAl8rk1RviWcDzhodYqH8/8idfu03EL4oY1QQ dGFcA2uFoZ7QFWj0RGaIwu/1jb46OWh1k5yvSnQTHsM6+CMblIuetAGxVb9goLo5I9pXnV tIthZSBRlGL8Kk69yYxhFY2ynOjEZ4Y= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="SORU/Lwz"; spf=pass (imf05.hostedemail.com: domain of glider@google.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=glider@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qv1-f54.google.com with SMTP id 6a1803df08f44-6b06e63d288so3021326d6.0 for ; Thu, 20 Jun 2024 01:36:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718872611; x=1719477411; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/OUYcIAPu8Sr/lzFQuuddP6LocZmaVvpQ1N05tgsbXo=; b=SORU/Lwz52RU/YRpbthQM6XVQ5btAqOIx1jmLKArWUQfHv0GV86CTUPpe6+6L/OGje b/OyD/kEulUc+O4giG7B2UWbSaDHhxvOkWackPVWa4CmwKSAFFvMzNTvdHRjGAS5TYFf PV7YT2T/vJsPfy3AFIaTJre2yvM+9sbVA1+DaM5LfvhqWqAHKSoFTnHzSaqKULu/3KSC gRHwRu3wbaAGYXyHa7SnI03pFxep7yvBHXR7bc3r+38uNtybXTdQkg6vUW3HeGS6v8Tc DF3NiK4R1DzcCIIP5EgwG+5zzC0Jsmjv/YPtSQeU9xffbQxOwasy5IJFHXbg0sUYhgkL W9Vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718872611; x=1719477411; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/OUYcIAPu8Sr/lzFQuuddP6LocZmaVvpQ1N05tgsbXo=; b=Hys9ioXZpeZNIxU+pGCNHbr/XqS9BxpQ8qtJDgwjARFWy9+ho4hQCTa8gBNBUX/Q27 eOLJ4TwRgAWQ70dRo6/q7vERAqHivZqEWu0z229mbq+a8fer5+Uj/TlEtC3FfGbkj83Z 79vajT0S/QPfDWA8OcehqtfGr1V7RWnfH7TzSOr1nu6W+oKAnRZNzrxZGZcXdOmH5FQj Z6LRRE/i54eDjnaanQDxSB5CbUTNPzvkRrHyLl6A9xze906jcpmFkUWCsOwVsHFxvwU4 ZwFoJh6X9b5LGaF21AHR3BuYee3trmwOep5BnY64UQFU3qm+MDscfvCIu4jHx3994Wyq 5g9Q== X-Forwarded-Encrypted: i=1; AJvYcCVlWpt1yHtMGv7rYNj+FlzjxJTHpYGc0kvSErwdllBOk06s+z6j6AM1rupAMHSU/Al4JEaEbCECDXfKDWweS49+Ib0= X-Gm-Message-State: AOJu0YxU5/qbRw8nFMVw0VhY4kRAy1OFdkdcoCAKJcstgAiYekCCE3uT 05C6AIP0qbXRbpLFYo6MVsCfkTl8mwVTcJPu/3ojmzTgDcPNNc9RfR2dzNqVETjESyOGiLOnNMD 77A89WX4ccl0NiCbKCB4oZ+1Uxlan77stTyCw X-Google-Smtp-Source: AGHT+IErmEp1x0YBOZTZJdlWJwFXEfsRyPhDcy8wmEVW3hTA6/SyMHr3il2LMbLMxYtDvHsm6Ik4jSDo32quxqAzw+g= X-Received: by 2002:a05:6214:4a42:b0:6b4:f761:f0b8 with SMTP id 6a1803df08f44-6b501dff5a9mr45755846d6.8.1718872610557; Thu, 20 Jun 2024 01:36:50 -0700 (PDT) MIME-Version: 1.0 References: <20240619154530.163232-1-iii@linux.ibm.com> <20240619154530.163232-34-iii@linux.ibm.com> In-Reply-To: <20240619154530.163232-34-iii@linux.ibm.com> From: Alexander Potapenko Date: Thu, 20 Jun 2024 10:36:12 +0200 Message-ID: Subject: Re: [PATCH v5 33/37] s390/uaccess: Add KMSAN support to put_user() and get_user() To: Ilya Leoshkevich Cc: Alexander Gordeev , Andrew Morton , Christoph Lameter , David Rientjes , Heiko Carstens , Joonsoo Kim , Marco Elver , Masami Hiramatsu , Pekka Enberg , Steven Rostedt , Vasily Gorbik , Vlastimil Babka , Christian Borntraeger , Dmitry Vyukov , Hyeonggon Yoo <42.hyeyoo@gmail.com>, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-s390@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Mark Rutland , Roman Gushchin , Sven Schnelle Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: ug4aheft3f534wpxw1jstw7dzcrr8md1 X-Rspamd-Queue-Id: 9BC99100010 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1718872611-844483 X-HE-Meta: U2FsdGVkX1+HLcZyPtcg7u0Qmo8UIMEWle3tu6gWwEiLx6UoKiLwahKSo4OF4WDUIRZAyfQjEjkLwHBSMZ9qzOEn3+8xgeO91G8k6jCf4JusfoQ54AthmL/OcIuLin4Whq6Ls8Sczb16JtZLSeLfUSRufNmpuOvJQkfuLbPAAEPgIbJGqps+W7AYJXE5mrgDs7GkWZXGQjTONW8mCqRiSP54aA6KaRuAnihoigUZXnRUv6kGG7NUrslOT36Cq47aZJZzepftYVB7V+A1yiuAnc89xj5PtvgtVkCu6hc/cb65TOFpoFYKExfU76lShbfItmgvdIVrn/lgK5nEtawKm/UhtU2O/W+JbhMcfWm6nx8ipTlwKCDghCBifuVysmTZQT7ayii10g2ZBL9LTf+duFUjpc8cqOImumnlDhBFXLrfGewhwXOZnxPEqAC66FgAmVIUljxdaao2ctI3mE5eQyn2YugnzkS644ULi1Y0i+/p9mBBLJg5MONVbHOUFxESld/rBItuRMHNhUnznEJy0rQVVmUZ11TXOrWrOPfm/QvxW5hL3++0c/tXfLq/fiXDXf3Zvj5iMCo5/CQqqiGeasDpwhpvzD0ncsAG437EvaZMGueMwPjRjKCUXCt2R3ItlfJa4G5c1lUnwSlSBcjWZGXV9kZffxwOogpCtNIbyzPCFMD5sOJX/HOqdgfuaW/NcdN4YrYefw5k5iNDK0QkCky8i2yAKmtfqTSP6HfO55KG+4gmI0KL0Ok4Ff24+MYP4TALnHCOO2lu5DDVMeWvoUwdFWZ6N22hir7/5STELC16oz9vBE56dfoudEKr8/W56zF0sftaY1CaUCrPvTFWIfksMFlrZFoqZLNRyvPvXupDhCjn5iwOivWI0Bz9uDCfMfmkvGGMtkw7JBBVZBZTiFfhSg+2u4tvdh+2y+hzKGapY8ElyohvsYeHb28RoWrA1SnFUjA1AIrb/32Qx2z IAQ6Mp7Q mLqYnggIQzD7BqRVaUc7uOdZThGYeqAKn9MFC5v+stUHL1WgLaIdeCf+DJI4xCQL+B+QwTqReETsZQyeYMBwmkSNOpF/BZOMkmFu01ZBHzYxCZNACNIOABmp9fgL3tC0KqgLlDu5EMI+3WhHji0esRsXx3x2v2JI0JnYlNUulmJ6VuOilCzrpyiOLCkmFz/y81BHyrix6e1Lw0A/OM4So5hM+W8PGrBT0+seU64zT1a1BBo82IQJf21easFCEfsMcccV38UUTxA/kNV8JSxR1RTPq6Xe8I35E1SzC3afG30Czy2jC3Eo3TAsyzyKPFkd4gFspuXnEoCVamnUmWyOjlN0ZT7Sj8Lr7eK8TQ2RR8568MaoANSws6tlCsCrGgyufL+CSdnDP07y/dbUjQbPDHn3H8cC5O5dsh4iFR9XqPYCC/Ohe4RlQ1fvJ4hs8FaRGFBX/d5rIuavMQB4= 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: List-Subscribe: List-Unsubscribe: On Wed, Jun 19, 2024 at 5:45=E2=80=AFPM Ilya Leoshkevich wrote: > > put_user() uses inline assembly with precise constraints, so Clang is > in principle capable of instrumenting it automatically. Unfortunately, > one of the constraints contains a dereferenced user pointer, and Clang > does not currently distinguish user and kernel pointers. Therefore > KMSAN attempts to access shadow for user pointers, which is not a right > thing to do. By the way, how does this problem manifest? I was expecting KMSAN to generate dummy shadow accesses in this case, and reading/writing 1-8 bytes from dummy shadow shouldn't be a problem. (On the other hand, not inlining the get_user/put_user functions is probably still faster than retrieving the dummy shadow, so I'm fine either way) > > An obvious fix to add __no_sanitize_memory to __put_user_fn() does not > work, since it's __always_inline. And __always_inline cannot be removed > due to the __put_user_bad() trick. > > A different obvious fix of using the "a" instead of the "+Q" constraint > degrades the code quality, which is very important here, since it's a > hot path. > > Instead, repurpose the __put_user_asm() macro to define > __put_user_{char,short,int,long}_noinstr() functions and mark them with > __no_sanitize_memory. For the non-KMSAN builds make them > __always_inline in order to keep the generated code quality. Also > define __put_user_{char,short,int,long}() functions, which call the > aforementioned ones and which *are* instrumented, because they call > KMSAN hooks, which may be implemented as macros. > > The same applies to get_user() as well. > > Acked-by: Heiko Carstens > Signed-off-by: Ilya Leoshkevich > --- > arch/s390/include/asm/uaccess.h | 111 +++++++++++++++++++++++--------- > 1 file changed, 79 insertions(+), 32 deletions(-) > > diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uacc= ess.h > index 81ae8a98e7ec..70f0edc00c2a 100644 > --- a/arch/s390/include/asm/uaccess.h > +++ b/arch/s390/include/asm/uaccess.h > @@ -78,13 +78,24 @@ union oac { > > int __noreturn __put_user_bad(void); > > -#define __put_user_asm(to, from, size) \ > -({ \ > +#ifdef CONFIG_KMSAN > +#define get_put_user_noinstr_attributes \ > + noinline __maybe_unused __no_sanitize_memory > +#else > +#define get_put_user_noinstr_attributes __always_inline > +#endif > + > +#define DEFINE_PUT_USER(type) \ > +static get_put_user_noinstr_attributes int \ > +__put_user_##type##_noinstr(unsigned type __user *to, \ > + unsigned type *from, \ > + unsigned long size) \ > +{ \ > union oac __oac_spec =3D { = \ > .oac1.as =3D PSW_BITS_AS_SECONDARY, = \ > .oac1.a =3D 1, = \ > }; \ > - int __rc; \ > + int rc; \ > \ > asm volatile( \ > " lr 0,%[spec]\n" \ > @@ -93,12 +104,28 @@ int __noreturn __put_user_bad(void); > "2:\n" \ > EX_TABLE_UA_STORE(0b, 2b, %[rc]) \ > EX_TABLE_UA_STORE(1b, 2b, %[rc]) \ > - : [rc] "=3D&d" (__rc), [_to] "+Q" (*(to)) = \ > + : [rc] "=3D&d" (rc), [_to] "+Q" (*(to)) = \ > : [_size] "d" (size), [_from] "Q" (*(from)), \ > [spec] "d" (__oac_spec.val) \ > : "cc", "0"); \ > - __rc; \ > -}) > + return rc; \ > +} \ > + \ > +static __always_inline int \ > +__put_user_##type(unsigned type __user *to, unsigned type *from, \ > + unsigned long size) \ > +{ \ > + int rc; \ > + \ > + rc =3D __put_user_##type##_noinstr(to, from, size); = \ > + instrument_put_user(*from, to, size); \ > + return rc; \ > +} > + > +DEFINE_PUT_USER(char); > +DEFINE_PUT_USER(short); > +DEFINE_PUT_USER(int); > +DEFINE_PUT_USER(long); > > static __always_inline int __put_user_fn(void *x, void __user *ptr, unsi= gned long size) > { > @@ -106,24 +133,24 @@ static __always_inline int __put_user_fn(void *x, v= oid __user *ptr, unsigned lon > > switch (size) { > case 1: > - rc =3D __put_user_asm((unsigned char __user *)ptr, > - (unsigned char *)x, > - size); > + rc =3D __put_user_char((unsigned char __user *)ptr, > + (unsigned char *)x, > + size); > break; > case 2: > - rc =3D __put_user_asm((unsigned short __user *)ptr, > - (unsigned short *)x, > - size); > + rc =3D __put_user_short((unsigned short __user *)ptr, > + (unsigned short *)x, > + size); > break; > case 4: > - rc =3D __put_user_asm((unsigned int __user *)ptr, > + rc =3D __put_user_int((unsigned int __user *)ptr, > (unsigned int *)x, > size); > break; > case 8: > - rc =3D __put_user_asm((unsigned long __user *)ptr, > - (unsigned long *)x, > - size); > + rc =3D __put_user_long((unsigned long __user *)ptr, > + (unsigned long *)x, > + size); > break; > default: > __put_user_bad(); > @@ -134,13 +161,17 @@ static __always_inline int __put_user_fn(void *x, v= oid __user *ptr, unsigned lon > > int __noreturn __get_user_bad(void); > > -#define __get_user_asm(to, from, size) \ > -({ \ > +#define DEFINE_GET_USER(type) \ > +static get_put_user_noinstr_attributes int \ > +__get_user_##type##_noinstr(unsigned type *to, \ > + unsigned type __user *from, \ > + unsigned long size) \ > +{ \ > union oac __oac_spec =3D { = \ > .oac2.as =3D PSW_BITS_AS_SECONDARY, = \ > .oac2.a =3D 1, = \ > }; \ > - int __rc; \ > + int rc; \ > \ > asm volatile( \ > " lr 0,%[spec]\n" \ > @@ -149,13 +180,29 @@ int __noreturn __get_user_bad(void); > "2:\n" \ > EX_TABLE_UA_LOAD_MEM(0b, 2b, %[rc], %[_to], %[_ksize]) \ > EX_TABLE_UA_LOAD_MEM(1b, 2b, %[rc], %[_to], %[_ksize]) \ > - : [rc] "=3D&d" (__rc), "=3DQ" (*(to)) = \ > + : [rc] "=3D&d" (rc), "=3DQ" (*(to)) = \ > : [_size] "d" (size), [_from] "Q" (*(from)), \ > [spec] "d" (__oac_spec.val), [_to] "a" (to), \ > [_ksize] "K" (size) \ > : "cc", "0"); \ > - __rc; \ > -}) > + return rc; \ > +} \ > + \ > +static __always_inline int \ > +__get_user_##type(unsigned type *to, unsigned type __user *from, \ > + unsigned long size) \ > +{ \ > + int rc; \ > + \ > + rc =3D __get_user_##type##_noinstr(to, from, size); = \ > + instrument_get_user(*to); \ > + return rc; \ > +} > + > +DEFINE_GET_USER(char); > +DEFINE_GET_USER(short); > +DEFINE_GET_USER(int); > +DEFINE_GET_USER(long); > > static __always_inline int __get_user_fn(void *x, const void __user *ptr= , unsigned long size) > { > @@ -163,24 +210,24 @@ static __always_inline int __get_user_fn(void *x, c= onst void __user *ptr, unsign > > switch (size) { > case 1: > - rc =3D __get_user_asm((unsigned char *)x, > - (unsigned char __user *)ptr, > - size); > + rc =3D __get_user_char((unsigned char *)x, > + (unsigned char __user *)ptr, > + size); > break; > case 2: > - rc =3D __get_user_asm((unsigned short *)x, > - (unsigned short __user *)ptr, > - size); > + rc =3D __get_user_short((unsigned short *)x, > + (unsigned short __user *)ptr, > + size); > break; > case 4: > - rc =3D __get_user_asm((unsigned int *)x, > + rc =3D __get_user_int((unsigned int *)x, > (unsigned int __user *)ptr, > size); > break; > case 8: > - rc =3D __get_user_asm((unsigned long *)x, > - (unsigned long __user *)ptr, > - size); > + rc =3D __get_user_long((unsigned long *)x, > + (unsigned long __user *)ptr, > + size); > break; > default: > __get_user_bad(); > -- > 2.45.1 > > -- > 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/20240619154530.163232-34-iii%40linux.ibm.com. --=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