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 CC494C4167B for ; Fri, 8 Dec 2023 15:26:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4915C6B0072; Fri, 8 Dec 2023 10:26:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 441326B0075; Fri, 8 Dec 2023 10:26:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2E2716B0096; Fri, 8 Dec 2023 10:26:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 18A046B0072 for ; Fri, 8 Dec 2023 10:26:25 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E1B608016E for ; Fri, 8 Dec 2023 15:26:24 +0000 (UTC) X-FDA: 81544027488.15.B9329A5 Received: from mail-qk1-f171.google.com (mail-qk1-f171.google.com [209.85.222.171]) by imf24.hostedemail.com (Postfix) with ESMTP id 1F77C18002B for ; Fri, 8 Dec 2023 15:26:22 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=kt6ZontU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of glider@google.com designates 209.85.222.171 as permitted sender) smtp.mailfrom=glider@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702049183; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=fmveRht4WTP9VCfRGY2TlBDNiNt4nqiRyGovDc1Drhw=; b=4KEd6tDAW3OFXI7gdOQSwDsmukHXFr4MLcHFvjxaaKaB9dJQTmqyN/hkxSkFYFxxAhCNOw Z4uNSBhB1U+3dKRJFFmpbLRelarS0Cqbrhcz0MXK371z46SvVSVC2mRbnSjUQ1buwlD5Ea dbuKEWI/IzS1LHdV9aZZ2j+N3Fgq7X0= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=kt6ZontU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of glider@google.com designates 209.85.222.171 as permitted sender) smtp.mailfrom=glider@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702049183; a=rsa-sha256; cv=none; b=PWaxAI/3GXa+X1DbY/eknlYdKFyQj6aaOyy5+QEvHe/ms7W1a/C9U2LhUTG4iW/un1ADk1 xagtlwZI2g+8M4CDe6Fj/bt+Vs59o0ad2CUIS3hWO7WkK8iFSLLOyBxfr/Drk2QQdWhYse jYnBTUHeU8G/0VBO+P9Zjp2DY6uvi/0= Received: by mail-qk1-f171.google.com with SMTP id af79cd13be357-77f524597c3so27196285a.3 for ; Fri, 08 Dec 2023 07:26:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702049182; x=1702653982; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=fmveRht4WTP9VCfRGY2TlBDNiNt4nqiRyGovDc1Drhw=; b=kt6ZontUwsxp3ZbIrct8IIkZhF85wWwfAG3bv4Vh2uFYjpgWzfxWH6XfF6a2sB15Ah hlSmOcknUkJdmIyuXKw+18Wg1m8yiftz2F51VTuQVVupv2qn8iQqQBx7WNZJk5vLwIJj 768Z5S9RxKffsoVXMPQBO7wvBmELcYXWAkwlLCnwq0LFtg4d3uL8VnauKs7W7ey8j56x Vgin2KdQPvy8y1oHucl6NWpDUvXUzXBBEIiohYO5tp9HSeEIfkETdhSsurcs3BRBuYhg r+6INj+AXA5vhCdvH5Zd/+DGSlFbnYNoVeqxfLp05jbxaV4ei0gRJ/tan7eS3c5dsJQT EMEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702049182; x=1702653982; h=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=fmveRht4WTP9VCfRGY2TlBDNiNt4nqiRyGovDc1Drhw=; b=fSKLmdgcSa6XA1jArWyPF9a+SoWoJh/66KZl2+qupIojmFlngnkmeSUcNJX9Jvllqf aF4TU/4evEz2dRhyVOvpq6SVQZ99KeILieplrQdTATeZ6pvUBg3Zhln3297HTygrhv1x U/i4nO1SgKmnEFDPqJ8XMImmW1ZeQzRM8TnwNrKRhrKKo/jo1raFdC6trbHafdxu4JLB pWlPVSBbN/10+A7L0dZ++8Y247qlkwInmL+ZVsG2GeU+lYnkwnf1i7fxmj+D70NepTiM naVch4Axm38nAD6JKglN9sWwoXQNbjqmw46KZAcOYGtOr0geSq1WYpOLaQE3514ktogK InCw== X-Gm-Message-State: AOJu0Yxx2TV+BhVh5VaueW/gqT4DSPJH3rpRlmCDAC2EgGu3juk9AN4n xinozA/7QjBYT7Z54X9K5rSmvba7JqQCiX2qOHrcWg== X-Google-Smtp-Source: AGHT+IFQWMwRSduRWrugzZXzhW4SP5JgRv6bQmddI9L+WLPciJYPF9LKHJS1ct3eyxJlLmbNonEC+PpCF/NYvyZxUy8= X-Received: by 2002:ad4:52eb:0:b0:67a:a721:82f8 with SMTP id p11-20020ad452eb000000b0067aa72182f8mr105489qvu.82.1702049182002; Fri, 08 Dec 2023 07:26:22 -0800 (PST) MIME-Version: 1.0 References: <20231121220155.1217090-1-iii@linux.ibm.com> <20231121220155.1217090-14-iii@linux.ibm.com> <69e7bc8e8c8a38c429a793e991e0509cb97a53e1.camel@linux.ibm.com> In-Reply-To: <69e7bc8e8c8a38c429a793e991e0509cb97a53e1.camel@linux.ibm.com> From: Alexander Potapenko Date: Fri, 8 Dec 2023 16:25:41 +0100 Message-ID: Subject: Re: [PATCH v2 13/33] kmsan: Introduce memset_no_sanitize_memory() 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" X-Rspamd-Queue-Id: 1F77C18002B X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: qo41uh7figc3yockj7msdcgmfztrwua9 X-HE-Tag: 1702049182-150339 X-HE-Meta: U2FsdGVkX1+X7nGeLcPclfzN0CMGrLJK9LqjinK0eE0e2FDteymXWoYQqCC1a+ocosbb3+wQqn2nBSmOj2jB79LfwvthwSb/s+YlBGFqCezHI/wt1IQHrNy79M6+W5aHjZE+6EBhPXrJFqSMWhk22CAFZZs5Pp5Uy7+/MV3H5jeuUkafIUEAqbkRoWRoy4dpZ7mbT/LaLIinopa3MZ3w09/tWCJhgHBqIoldfZF2jOwlfTrhfmzH9H4o2hMhvIZjuGb9+jy0CNrFEyHFkHMV//lLRQWXmVhteQN3oYJ66gGpyZqw0Ntz88vreKiyIITs8iG2/B8tZLqUBl3Dv0rQ4RFUd6B8ajPKyLSxsQuf787WQmRcCn/jQXQbWj2GsRGjjA7dhqoxLWiLdh5IWGpHGjtrqUPfmX5aGabWVgW5EmduXBKnJ1f2D8V2FqyIvugj2eVumQAU3u4DqorL5CCOgbDzHBENeyCN2xZk2cdMbWnVpfs+48Xh+IYnR8le7uB3IyLxyzz8zyu7RMf8VZ0F9nZ1JfdmRrOYq1AAjBqqVFcIBTYj7kUsn4OUvME8K7lkd8+Cgtd4yf8Ox03q1HFCqJrDCEQXDYFt7zqohRxTkNGyF87/n8KWJlSv2bKO7ZwceI9EP/5TCVEcVHeoOyOUhisbEpVenyJW44FxDEAF/wC6h2u5l/GA+f2ZmTGd6Gys0f4+gewgSROwX4fJ7FQRzUYjGBWsi6zARPWXk9mONDId3/mRw+kFT2Y3kvNWiaRmgCawC3T5BPCpo3vIU7pdighTVlnEZNW/py8KjD1y8X7HTFb1ebN/ky/UFkpfY4mFjXAfLkax83MClknlqN3oPoOvVpis56MpOjqdTy/nxoRydMsKwvheSotoVgIY6RYoYn9mdkSdLfv/xj0k62Ck9UuPQBwEzaDC2yJ46xz8+dAhWUsmtgrOd+xBy8VvlrYm/Usn/MFJtUWCjkVGhWH 0B4X3RYe FbY2Tb90nEhc75uY2jT2zepaX4FN1OQ+wNX8MF+tR/YNRxA7MYxzg17enfjHnrFq0v1nWrR1wDIIWTNeaW8DuhSA0mey3CFB967+5ukp0PYpY0b/i8BfmfuKJ/qVAK14fAVi/o7cN+xus9oKj61bti+lviO7+3WQUq8HcFt2GDp++vfCgq/tp1FZfJRs98ZvvU7q+N2hLsDH/oWJrXVO+q2mjMlvKMibQHcMZ2XyYOJMWhZNeylFwLqlZhFt5sLsIiHNP9DwDW0C8lvXzIm3KfQjCD+gUubgJOPgFqisranNQfPnV6ufrblzfAFUH/ymgxiQj6XnPtcU5htLoKk+DxjQpB4hmVygK0rYKUwk4ZqKUtXJ6EssHC/pv7z9eWpi6m18A1JV/Nppu6FiMRWmq541Z5g== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000008, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: > A problem with __memset() is that, at least for me, it always ends > up being a call. There is a use case where we need to write only 1 > byte, so I thought that introducing a call there (when compiling > without KMSAN) would be unacceptable. Wonder what happens with that use case if we e.g. build with fortify-source. Calling memset() for a single byte might be indicating the code is not hot. > > ... > > > > > +__no_sanitize_memory > > > +static inline void *memset_no_sanitize_memory(void *s, int c, > > > size_t n) > > > +{ > > > + return memset(s, c, n); > > > +} > > > > I think depending on the compiler optimizations this might end up > > being a call to normal memset, that would still change the shadow > > bytes. > > Interesting, do you have some specific scenario in mind? I vaguely > remember that in the past there were cases when sanitizer annotations > were lost after inlining, but I thought they were sorted out? Sanitizer annotations are indeed lost after inlining, and we cannot do much about that. They are implemented using function attributes, and if a function dissolves after inlining, we cannot possibly know which instructions belonged to it. Consider the following example (also available at https://godbolt.org/z/5r7817G8e): ================================== void *kmalloc(int size); __attribute__((no_sanitize("kernel-memory"))) __attribute__((always_inline)) static void *memset_nosanitize(void *s, int c, int n) { return __builtin_memset(s, c, n); } void *do_something_nosanitize(int size) { void *ptr = kmalloc(size); memset_nosanitize(ptr, 0, size); return ptr; } void *do_something_sanitize(int size) { void *ptr = kmalloc(size); __builtin_memset(ptr, 0, size); return ptr; } ================================== If memset_nosanitize() has __attribute__((always_inline)), the compiler generates the same LLVM IR calling __msan_memset() for both do_something_nosanitize() and do_something_sanitize(). If we comment out this attribute, do_something_nosanitize() calls memset_nosanitize(), which doesn't have the sanitize_memory attribute. But even now __builtin_memset() is still calling __msan_memset(), because __attribute__((no_sanitize("kernel-memory"))) somewhat counterintuitively still preserves some instrumentation (see include/linux/compiler-clang.h for details). Replacing __attribute__((no_sanitize("kernel-memory"))) with __attribute__((disable_sanitizer_instrumentation)) fixes this situation: define internal fastcc noundef ptr @memset_nosanitize(void*, int, int)(ptr noundef returned writeonly %s, i32 noundef %n) unnamed_addr #2 { entry: %conv = sext i32 %n to i64 tail call void @llvm.memset.p0.i64(ptr align 1 %s, i8 0, i64 %conv, i1 false) ret ptr %s } > > And, in any case, if this were to happen, would not it be considered a > compiler bug that needs fixing there, and not in the kernel? As stated above, I don't think this is more or less working as intended. If we really want the ability to inline __memset(), we could transform it into memset() in non-sanitizer builds, but perhaps having a call is also acceptable?