Message ID | 20231121220155.1217090-14-iii@linux.ibm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2b07:b0:403:3b70:6f57 with SMTP id io7csp949025vqb; Tue, 21 Nov 2023 14:19:30 -0800 (PST) X-Google-Smtp-Source: AGHT+IHlo2eDK+FTFY4J38CJ5sjLTZIKHTYlOng+UuPrxL0KTZGuTRzIQLbvSzUV9+sr0HpfdXL3 X-Received: by 2002:a17:90b:4ac9:b0:285:24f1:8706 with SMTP id mh9-20020a17090b4ac900b0028524f18706mr567991pjb.14.1700605169738; Tue, 21 Nov 2023 14:19:29 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700605169; cv=none; d=google.com; s=arc-20160816; b=J026wDVfLnsaD84sRAA5wMXvvYu/Vbgv19ZT9lIgafuqdYZWYAuCbArgI9qVLmLqXS WCbAMVWH0sOCbD0Y69FYvNXImnNcK4bIHHndeATYfFCGHnrikydO3XzjhyFyz0lv2ong CCUN/hSe30AocMcLJNp4SrsY7Gcy/3ZpG04Bd4tU/T2eDOz/Ow1AJ7wdpnctmYTCq5M0 JYDyRspQ1cPM8TR8INtwmkZ7ZqSlT7hWP4J/QO3oDmKJ8F606Xel6iBRq5y7FvQozgB5 GVpRm50RXlT66EkqjflS2++by0eEIqKsVPeoVCp5/HVHeLem3C6l9IFHXeCa1K3kAtqu V2EQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=w3qU2hLQPViAb6IauvgW6Gr8yASYdUVClk+0doiSRWs=; fh=TQATEbdDZNcnk8L2eDP6eFL9HlexFaHIexhR1TH2IlY=; b=J+joomeR677qhmWFKxQyaaxGX8RJI3qo5P64/Lanl0S7U3r0K4/I4eZ0gd2Difnqz4 h87z3Js3nryyk4Lqmhoobfq8F85bSbMPcToYdqDufkuMVPrTbB1pbD5HUh2+3hXpesyA iprchzmARFt4yCIcBut0x/aalBS8kCzEpMDj/G9QccZ85FUCcwjCxmF7dM2cnsbL/ga2 48Y1AKOiRag7gfCxyWD0b5LKUhLv0mDT1Ziik/jD+4+YumU9K0idfiEvELomEPviHqeM 8JfJFZZrrCEFd0WP3bpg/UEJarIh04M9QKaX0ZJ/DVKWwewlsZ5eI5KMzdsBn/6H4ItY Xx3g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=SxOEgF2k; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=ibm.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id i14-20020a17090ad34e00b002847ae0871fsi47225pjx.125.2023.11.21.14.19.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Nov 2023 14:19:29 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=SxOEgF2k; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=ibm.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 069A0806503F; Tue, 21 Nov 2023 14:17:01 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234657AbjKUWQi (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Tue, 21 Nov 2023 17:16:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36604 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234364AbjKUWQf (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 21 Nov 2023 17:16:35 -0500 Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1D75626AF; Tue, 21 Nov 2023 14:06:29 -0800 (PST) Received: from pps.filterd (m0353723.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3ALLlRZr007601; Tue, 21 Nov 2023 22:06:14 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=w3qU2hLQPViAb6IauvgW6Gr8yASYdUVClk+0doiSRWs=; b=SxOEgF2kgmmUva2CthNSWR18hHL8V4G5GHjToXGpDK/yYBfUuPHnzVjp6qNC9pAGfKpO K2+lgfDkxttYSB9vTbXjqgBmmRXRy6oXVHZtryhxrZz3lWLfzk2vNimfBcotLSQeXw/G Y8x6+UdIM5/AbZ82UvyZk6E+emRBlFU9EYDbBFPnjSFdB5z8jFs+cSDf1C4EGSVM/WkK T7YEMKc1jcCGwyMg/lBVqGVpNxTghdXfdY+Ody2Rpapaq60KrPQxSqpyoerHY620j+Y3 IG3WiEBDPY5f2sjprKeX7pYV+RJlZYOWC5Fu50phO4x7XsMn6fmJlA7JvM/7rzhSTqYx Kg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uh4dw10s7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Nov 2023 22:06:14 +0000 Received: from m0353723.ppops.net (m0353723.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3ALLQnKe012479; Tue, 21 Nov 2023 22:06:13 GMT Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uh4dw10pk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Nov 2023 22:06:13 +0000 Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3ALLnSib004674; Tue, 21 Nov 2023 22:02:31 GMT Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3uf7yykvgb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 21 Nov 2023 22:02:30 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 3ALM2Rff17629862 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 21 Nov 2023 22:02:28 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CDE9720065; Tue, 21 Nov 2023 22:02:27 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 67E6D20063; Tue, 21 Nov 2023 22:02:26 +0000 (GMT) Received: from heavy.boeblingen.de.ibm.com (unknown [9.179.23.98]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Tue, 21 Nov 2023 22:02:26 +0000 (GMT) From: Ilya Leoshkevich <iii@linux.ibm.com> To: Alexander Gordeev <agordeev@linux.ibm.com>, Alexander Potapenko <glider@google.com>, Andrew Morton <akpm@linux-foundation.org>, Christoph Lameter <cl@linux.com>, David Rientjes <rientjes@google.com>, Heiko Carstens <hca@linux.ibm.com>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, Marco Elver <elver@google.com>, Masami Hiramatsu <mhiramat@kernel.org>, Pekka Enberg <penberg@kernel.org>, Steven Rostedt <rostedt@goodmis.org>, Vasily Gorbik <gor@linux.ibm.com>, Vlastimil Babka <vbabka@suse.cz> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>, Dmitry Vyukov <dvyukov@google.com>, 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 <mark.rutland@arm.com>, Roman Gushchin <roman.gushchin@linux.dev>, Sven Schnelle <svens@linux.ibm.com>, Ilya Leoshkevich <iii@linux.ibm.com> Subject: [PATCH v2 13/33] kmsan: Introduce memset_no_sanitize_memory() Date: Tue, 21 Nov 2023 23:01:07 +0100 Message-ID: <20231121220155.1217090-14-iii@linux.ibm.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231121220155.1217090-1-iii@linux.ibm.com> References: <20231121220155.1217090-1-iii@linux.ibm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: AjHKs9P_s-EjY6PEk-BnWe3k9pw_FvZj X-Proofpoint-ORIG-GUID: P-jmXZ89sI19Qp3IwQq4whgKQV4vNRwT X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-21_12,2023-11-21_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 mlxlogscore=799 spamscore=0 suspectscore=0 phishscore=0 priorityscore=1501 malwarescore=0 clxscore=1015 impostorscore=0 adultscore=0 bulkscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311060000 definitions=main-2311210172 X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 21 Nov 2023 14:17:01 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783213766453828715 X-GMAIL-MSGID: 1783213766453828715 |
Series |
kmsan: Enable on s390
|
|
Commit Message
Ilya Leoshkevich
Nov. 21, 2023, 10:01 p.m. UTC
Add a wrapper for memset() that prevents unpoisoning. This is useful
for filling memory allocator redzones.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
include/linux/kmsan.h | 9 +++++++++
1 file changed, 9 insertions(+)
Comments
On Tue, Nov 21, 2023 at 11:06 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Add a wrapper for memset() that prevents unpoisoning. We have __memset() already, won't it work for this case? On the other hand, I am not sure you want to preserve the redzone in its previous state (unless it's known to be poisoned). You might consider explicitly unpoisoning the redzone instead. ... > +__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.
On Fri, 2023-12-08 at 14:48 +0100, Alexander Potapenko wrote: > On Tue, Nov 21, 2023 at 11:06 PM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > Add a wrapper for memset() that prevents unpoisoning. > > We have __memset() already, won't it work for this case? 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. > On the other hand, I am not sure you want to preserve the redzone in > its previous state (unless it's known to be poisoned). That's exactly the problem with unpoisoning: it removes the distinction between a new allocation and a UAF. > You might consider explicitly unpoisoning the redzone instead. That was my first attempt, but it resulted in test failures due to the above. > ... > > > +__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? 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?
> 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?
On Fri, 2023-12-08 at 16:25 +0100, Alexander Potapenko wrote: > > 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. The original code has a simple assignment. Here is the relevant diff: if (s->flags & __OBJECT_POISON) { - memset(p, POISON_FREE, poison_size - 1); - p[poison_size - 1] = POISON_END; + memset_no_sanitize_memory(p, POISON_FREE, poison_size - 1); + memset_no_sanitize_memory(p + poison_size - 1, POISON_END, 1); } [...] > 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? Thanks for the detailed explanation and analysis. I will post a version with a __memset() and let the slab maintainers decide if the additional overhead is acceptable.
On Wed, 2023-12-13 at 02:31 +0100, Ilya Leoshkevich wrote: > On Fri, 2023-12-08 at 16:25 +0100, Alexander Potapenko wrote: > > > 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. [...] > > 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? > > Thanks for the detailed explanation and analysis. I will post > a version with a __memset() and let the slab maintainers decide if > the additional overhead is acceptable. I noticed I had the same problem in the get_user()/put_user() and check_canary() patches. The annotation being silently ignored is never what a programmer intends, so what do you think about adding noinline to __no_kmsan_checks and __no_sanitize_memory?
diff --git a/include/linux/kmsan.h b/include/linux/kmsan.h index ff8fd95733fa..439df72c8dc6 100644 --- a/include/linux/kmsan.h +++ b/include/linux/kmsan.h @@ -345,4 +345,13 @@ static inline void *kmsan_get_metadata(void *addr, bool is_origin) #endif +/** + * memset_no_sanitize_memory() - memset() without the KMSAN instrumentation. + */ +__no_sanitize_memory +static inline void *memset_no_sanitize_memory(void *s, int c, size_t n) +{ + return memset(s, c, n); +} + #endif /* _LINUX_KMSAN_H */