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 DD8E6C433FE for ; Fri, 18 Nov 2022 10:32:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4D1536B0071; Fri, 18 Nov 2022 05:32:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 45B218E0002; Fri, 18 Nov 2022 05:32:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D4008E0001; Fri, 18 Nov 2022 05:32:41 -0500 (EST) 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 171066B0071 for ; Fri, 18 Nov 2022 05:32:41 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id D29821A02AB for ; Fri, 18 Nov 2022 10:32:40 +0000 (UTC) X-FDA: 80146199280.04.5044D91 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf22.hostedemail.com (Postfix) with ESMTP id 33356C0010 for ; Fri, 18 Nov 2022 10:32:38 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 7D77F1F890; Fri, 18 Nov 2022 10:32:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1668767557; h=from:from:reply-to: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; bh=XzHJXeK3Q2/Bx013Ighw0ir5MysszSUlvQpViISJqBs=; b=g8xwvhAwqhbxisPUlVbujWUoHsv5l3uONqGun9DeIQiCAadM+i1rYsCwZIaj4H4PpmIkgI v0pg0R+CThhbSKC5VAVA1zhrmiDrRN84oYCMC+vbKDlunUICpi8B7IdDTW2/+eKdRxmt/s Yx8YBG1i1FbxFUiL3sXhWVkWHeEWA2U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1668767557; h=from:from:reply-to: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; bh=XzHJXeK3Q2/Bx013Ighw0ir5MysszSUlvQpViISJqBs=; b=IGKMdrU51wRPjSgtt3cDgq7HIyboRyHc/nsdePlag/9cMrpqnR2NQkqLjJOG1FY+dFyZeS Pge36YS2pYY05lDg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 3C66113A66; Fri, 18 Nov 2022 10:32:37 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id XG5tDUVfd2NwRgAAMHmgww (envelope-from ); Fri, 18 Nov 2022 10:32:37 +0000 Message-ID: <230127af-6c71-e51e-41a4-aa9547c2c847@suse.cz> Date: Fri, 18 Nov 2022 11:32:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v2] mm: Make ksize() a reporting-only function Content-Language: en-US To: Kees Cook , Andrey Konovalov 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, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org References: <20221118035656.gonna.698-kees@kernel.org> From: Vlastimil Babka In-Reply-To: <20221118035656.gonna.698-kees@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=g8xwvhAw; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=IGKMdrU5; spf=pass (imf22.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668767559; a=rsa-sha256; cv=none; b=WVwhU0O2/NTmbFz89LLaqBH9DMtWChBdMt9H6HnPVY9wdxQd3rVu2HGTYaDMx32FKbcciA lYRZNtBWuNsM2BvdH8HtL2QNQcR48tTBc8fQZMO7dcxxYzdIZ0XLDr61z/I5gUZ7kDE8GC V8RWPhBAExXBky4mOy1yyEtMQIIe62w= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668767559; 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=XzHJXeK3Q2/Bx013Ighw0ir5MysszSUlvQpViISJqBs=; b=Ss7T+ujuSb1jzhcWhCnkPF9IE1KKUpBKw1GShOoSvQTi/1zZHwmZX469aCFzg2EeC+/y51 9tz5NCtGiGsGSzdsqAaTbKB2ehV8cT2OmbH6d187TmM7FaUsO38m9PPGGx2f0G9LL+EJk+ iF5UrHzp7d6Sld2u1+LY15HorpUGab0= Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=g8xwvhAw; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=IGKMdrU5; spf=pass (imf22.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none X-Rspamd-Server: rspam02 X-Rspam-User: X-Stat-Signature: tuksdchh3ygije3ncemfudqdey17sqgh X-Rspamd-Queue-Id: 33356C0010 X-HE-Tag: 1668767558-483663 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 11/18/22 04:56, Kees Cook wrote: > With all "silently resizing" callers of ksize() refactored, remove the At cursory look seems it's true now in -next (but not mainline?) can you confirm? That would probably be safe enough to have slab.git expose this to -next now and time a PR appropriately in the next merge window? > 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]); > > 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); >