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 0D943C4321E for ; Sat, 26 Nov 2022 17:04:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 04ADF6B0074; Sat, 26 Nov 2022 12:04:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F3CF16B0075; Sat, 26 Nov 2022 12:04:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DDDEB6B0078; Sat, 26 Nov 2022 12:04:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CB4946B0074 for ; Sat, 26 Nov 2022 12:04:53 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8C0AAAB1B4 for ; Sat, 26 Nov 2022 17:04:53 +0000 (UTC) X-FDA: 80176218066.07.01E9AA7 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) by imf03.hostedemail.com (Postfix) with ESMTP id 1C39F20011 for ; Sat, 26 Nov 2022 17:04:51 +0000 (UTC) Received: by mail-pf1-f170.google.com with SMTP id w129so6705953pfb.5 for ; Sat, 26 Nov 2022 09:04:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=uKmnIaloULHriFcopQa51nxjENCmgGQKjpIIPfSTFtA=; b=cUl3qd8uHgMRnh/jnbftDT5GuOqOGJykCT0pya8ESdkiNfooRYdYuiSOuFbi6w33q0 z54HxlT4PtUYM/1qhIPEZBiXQJ7WSuwNx2xlQgP6BKp21xw8RQoecof64jc7AaarEseG 7ooKsL4oeFBlvE3/IBHNRYQ6aDhlmVNYVB4pnJnhZyGAHQFKx1MYNc/wsFicJ+hqf5aZ vruBYyyEKpuKKA5K53JKW+sitwUlXyLTjv5Z0OVBb/OOSxnU7i6Y//RxtAJYK4EYlaLO tTPxVFL1iOHrADsTKZrnRNjFIOGluQkFeHLIsV05MxwKLlZcaJztx8K6uK1d/Zb/5RWM FvRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=uKmnIaloULHriFcopQa51nxjENCmgGQKjpIIPfSTFtA=; b=0dsLeDNqZ0yNwp9lX+kJI/e4jV63ZlMIEa3wQt0Ee7U849mFQXbOK3c+aGvlC/Pui0 OgBXHkQEZE67d/31InHrCN64I5n4YugptdqyOQ1UPB+oQwiJtKYlNoz0P6cSt7cgpxbt V2kBqqkZ5v794wLJk+7XAXSjLga8sB3PVrA4bLBUHQo/RphimK3QggFt31wdonvCMy+A Yag6w5LCXiT1ddHMNJycdvGLAdjpsRPzlRdaGHzXOqrCVnyRJmblUm+n7kOt4UmITMky bHOMXLuvYqNVVkZ9rtfUqOoh/OZw/9/02MCM+ECx+rWN974vyDpq3SgBu/6VLmQo52IZ xq+Q== X-Gm-Message-State: ANoB5pkp1frCZWWH2S04PAMuEpZjm/qXX/wOpqSETEx3Z+Jip7Mgz4hZ 5RTwszlLGDBUokWNgL3Qj/dMAm4/Kh/Te5XCsP8= X-Google-Smtp-Source: AA0mqf7LcR92cxqWYPsjOyXeEfk+DWYLmX6aavVi7/wkWWex0aMbJRaW7PDJ8WhIgUqw7J233DDg7osc4GCEU+Cq+WU= X-Received: by 2002:a63:1659:0:b0:477:98cc:3cfe with SMTP id 25-20020a631659000000b0047798cc3cfemr20169345pgw.508.1669482290899; Sat, 26 Nov 2022 09:04:50 -0800 (PST) MIME-Version: 1.0 References: <20221118035656.gonna.698-kees@kernel.org> In-Reply-To: <20221118035656.gonna.698-kees@kernel.org> From: Andrey Konovalov Date: Sat, 26 Nov 2022 18:04:39 +0100 Message-ID: Subject: Re: [PATCH v2] mm: Make ksize() a reporting-only function To: Kees Cook Cc: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Vincenzo Frascino , linux-mm@kvack.org, kasan-dev@googlegroups.com, Vlastimil Babka , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=cUl3qd8u; spf=pass (imf03.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.210.170 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669482292; a=rsa-sha256; cv=none; b=yMkSQZRzKBTye5xig32HAvX3AlXwlv6/p0kmWxNVMKHnjpEbtHI53mY2MrkBoBF5I7LOJ+ 9LbQddQweQfM+7j31sKKErojQ6enFjGZkrgxVfM1sltH9IvKMkGCZ0RIp7r08gj1rphFf0 /9Cg96OAkW3ROl4VbADWzwc1p+178Qs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669482292; 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=uKmnIaloULHriFcopQa51nxjENCmgGQKjpIIPfSTFtA=; b=8ECxDcmMqBrMVfSqmAbhl2pKNowuflRT25xJvskQBzkaXa2zJngLL8eKDjt8tIewuS1slO Wd+eFEf8ju1ZeqCG9Onsl30HNqhCjHTXtVmHMS3APyDTYVg0oq/vi/n3rigELgiH98lqGU 5SmFNUj5h9ElWuCKYiA3FZ0IzsGgGeQ= Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=cUl3qd8u; spf=pass (imf03.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.210.170 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Queue-Id: 1C39F20011 X-Rspamd-Server: rspam01 X-Stat-Signature: f8xs98ahwj7uw4qmzn96hi1yaf6w3map X-HE-Tag: 1669482291-930387 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, Nov 18, 2022 at 4:57 AM Kees Cook wrote: > > With all "silently resizing" callers of ksize() refactored, remove the > logic in ksize() that would allow it to be used to effectively change > the size of an allocation (bypassing __alloc_size hints, etc). Users > wanting this feature need to either use kmalloc_size_roundup() before an > allocation, or use krealloc() directly. > > For kfree_sensitive(), move the unpoisoning logic inline. Replace the > some of the partially open-coded ksize() in __do_krealloc with ksize() > now that it doesn't perform unpoisoning. > > Adjust the KUnit tests to match the new ksize() behavior. > > Cc: Andrey Konovalov > Cc: Christoph Lameter > Cc: Pekka Enberg > Cc: David Rientjes > Cc: Joonsoo Kim > Cc: Andrew Morton > Cc: Roman Gushchin > Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com> > Cc: Andrey Ryabinin > Cc: Alexander Potapenko > Cc: Dmitry Vyukov > Cc: Vincenzo Frascino > Cc: linux-mm@kvack.org > Cc: kasan-dev@googlegroups.com > Acked-by: Vlastimil Babka > Signed-off-by: Kees Cook > --- > v2: > - improve kunit test precision (andreyknvl) > - add Ack (vbabka) > v1: https://lore.kernel.org/all/20221022180455.never.023-kees@kernel.org > --- > mm/kasan/kasan_test.c | 14 +++++++++----- > mm/slab_common.c | 26 ++++++++++---------------- > 2 files changed, 19 insertions(+), 21 deletions(-) > > diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c > index 7502f03c807c..fc4b22916587 100644 > --- a/mm/kasan/kasan_test.c > +++ b/mm/kasan/kasan_test.c > @@ -821,7 +821,7 @@ static void kasan_global_oob_left(struct kunit *test) > KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p); > } > > -/* Check that ksize() makes the whole object accessible. */ > +/* Check that ksize() does NOT unpoison whole object. */ > static void ksize_unpoisons_memory(struct kunit *test) > { > char *ptr; > @@ -829,15 +829,19 @@ static void ksize_unpoisons_memory(struct kunit *test) > > ptr = kmalloc(size, GFP_KERNEL); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > + > real_size = ksize(ptr); > + KUNIT_EXPECT_GT(test, real_size, size); > > OPTIMIZER_HIDE_VAR(ptr); > > - /* This access shouldn't trigger a KASAN report. */ > - ptr[size] = 'x'; > + /* These accesses shouldn't trigger a KASAN report. */ > + ptr[0] = 'x'; > + ptr[size - 1] = 'x'; > > - /* This one must. */ > - KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[real_size]); > + /* These must trigger a KASAN report. */ > + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[size]); > + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[real_size - 1]); Hi Kees, I just realized there's an issue here with the tag-based modes, as they align the unpoisoned area to 16 bytes. One solution would be to change the allocation size to 128 - KASAN_GRANULE_SIZE - 5, the same way kmalloc_oob_right test does it, so that the last 16-byte granule won't get unpoisoned for the tag-based modes. And then check that the ptr[size] access fails only for the Generic mode. Thanks! > > kfree(ptr); > } > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 8276022f0da4..27caa57af070 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -1335,11 +1335,11 @@ __do_krealloc(const void *p, size_t new_size, gfp_t flags) > void *ret; > size_t ks; > > - /* Don't use instrumented ksize to allow precise KASAN poisoning. */ > + /* Check for double-free before calling ksize. */ > if (likely(!ZERO_OR_NULL_PTR(p))) { > if (!kasan_check_byte(p)) > return NULL; > - ks = kfence_ksize(p) ?: __ksize(p); > + ks = ksize(p); > } else > ks = 0; > > @@ -1407,21 +1407,21 @@ void kfree_sensitive(const void *p) > void *mem = (void *)p; > > ks = ksize(mem); > - if (ks) > + if (ks) { > + kasan_unpoison_range(mem, ks); > memzero_explicit(mem, ks); > + } > kfree(mem); > } > EXPORT_SYMBOL(kfree_sensitive); > > size_t ksize(const void *objp) > { > - size_t size; > - > /* > - * We need to first check that the pointer to the object is valid, and > - * only then unpoison the memory. The report printed from ksize() is > - * more useful, then when it's printed later when the behaviour could > - * be undefined due to a potential use-after-free or double-free. > + * We need to first check that the pointer to the object is valid. > + * The KASAN report printed from ksize() is more useful, then when > + * it's printed later when the behaviour could be undefined due to > + * a potential use-after-free or double-free. > * > * We use kasan_check_byte(), which is supported for the hardware > * tag-based KASAN mode, unlike kasan_check_read/write(). > @@ -1435,13 +1435,7 @@ size_t ksize(const void *objp) > if (unlikely(ZERO_OR_NULL_PTR(objp)) || !kasan_check_byte(objp)) > return 0; > > - size = kfence_ksize(objp) ?: __ksize(objp); > - /* > - * We assume that ksize callers could use whole allocated area, > - * so we need to unpoison this area. > - */ > - kasan_unpoison_range(objp, size); > - return size; > + return kfence_ksize(objp) ?: __ksize(objp); > } > EXPORT_SYMBOL(ksize); > > -- > 2.34.1 >