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 9E058ECE582 for ; Tue, 10 Sep 2024 10:06:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1B33B8D0051; Tue, 10 Sep 2024 06:06:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 163678D0002; Tue, 10 Sep 2024 06:06:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 051C88D0051; Tue, 10 Sep 2024 06:06:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id DE7268D0002 for ; Tue, 10 Sep 2024 06:06:15 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8890941585 for ; Tue, 10 Sep 2024 10:06:15 +0000 (UTC) X-FDA: 82548398310.10.6575DA3 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf19.hostedemail.com (Postfix) with ESMTP id DFF671A0004 for ; Tue, 10 Sep 2024 10:06:13 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cP9NOIs2; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf19.hostedemail.com: domain of dakr@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=dakr@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725962770; a=rsa-sha256; cv=none; b=oJjElLSOIvjPPGYllSfaJkNLkIEUO1A1edyHqkXrUT1Fq2NUwA6tu5MXqglvdomfJGEk4X Yq1ZZiatEdORZVGGZMjiHfYqfwvD3ObzqiMH658K1ZIYoNnY2wco/eVaXXwdN8I19ax1Tp DOLh+Oq5r50jw+FPIE3yZVXHXxZ5p58= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cP9NOIs2; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf19.hostedemail.com: domain of dakr@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=dakr@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725962770; 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=Tiri5qlTCVkug9N1Dqz9t/5YibWAEAfeNKGEKHFZkcw=; b=ghJ6V7b5qA+PGIoxDJ4l5U9iBFEwf/77ThaDi8FdxakIgCxJrRvFVUhN0Kakkvb/6IUQDr c/e9NpFTm7nDNmP8OQ6JWcnu4hnERiaVMMyhnbzyjvysPYQhwj6NoL6LHrBWWQAJ440sBz g9beBN6vbhXFKjcq252ZD+LFArlWBsU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 7EAA9A444E8; Tue, 10 Sep 2024 10:06:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 77D24C4CEC6; Tue, 10 Sep 2024 10:06:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1725962771; bh=er8gmKzN5Faog+t3Xxq4cRtUP9mJj5HchMtTy2TFXrw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cP9NOIs2J4fY6fWtmL/u5awHpTS3jgROfQOdd1iyGEcmgcjvmGbn2ZO2i6Squevc9 Xk4K/Veu34zbmRuv+Pi0+TMzxqElg8LbBhBdx1cUcHHEqJ1JHdmHEeggzNBFxw6pf/ awKuxU79p0FKmlhREpUsoj3EhIpvqmJPSS29125+5dBO2S2DCkWCM9zKxde36DnoUA BrSvXfDfvc39eq4ZpYouTc1Mnr1hOpNibNp1QIQuXNGxzOPAmVYwLR+TLKUo61XK5F XyxP0jx4t0MaGcTKjIjrtVwcvMROQ45kMVWheMcJBJVkFjWuKIUKu4V7qhiS7ecV6u Cby1zk7tMtz7g== Date: Tue, 10 Sep 2024 12:06:05 +0200 From: Danilo Krummrich To: Feng Tang Cc: Vlastimil Babka , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Andrey Konovalov , Marco Elver , Shuah Khan , David Gow , linux-mm@kvack.org, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/5] mm/slub: Improve redzone check and zeroing for krealloc() Message-ID: References: <20240909012958.913438-1-feng.tang@intel.com> <20240909012958.913438-4-feng.tang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240909012958.913438-4-feng.tang@intel.com> X-Rspam-User: X-Stat-Signature: fykh4yroy9tm8itzn1o931nqrfrsu6wq X-Rspamd-Queue-Id: DFF671A0004 X-Rspamd-Server: rspam02 X-HE-Tag: 1725962773-402877 X-HE-Meta: U2FsdGVkX19TgExSqczbvYrve2SardvvNbxzvJ3XR1S5fNNTAp35M7M6CAB6uEz56tyukyv4UO0qc9vHFMcw8CDosv5syM8y9Vl4Va/4tRuHamcpRM+oI1e4QOmorHbQzDRzvcVoNyBRUsocY2ctW6yxwtDdDWCvk158o7qgkIiQ7Fg8MtikdfIOaxhS6+e7vxMmQTlDo87iwPY6AUp4eOfaQRtWAslxS+N7v/t3yPI7uFHaciA6G3whaHU495r5Uh6eP+2mvK0MQ4SAXQX+lT+H+YoCkKTX9cMS4OfDjclNyq/xhjPb/OkP2XBfKdelNuou1eQ3gqmGbo5O1Chcraj7LYJk46BrSAr1WbsPMPNTr6gKRNL1A55bYPz6Uq+LAUhNE60yxehU0owX5A7u7CjjW98dZmkQJ/iuCFQZ9Touwetz+JWy7nMx6cP0p676CejlvmNwsHLu/zS4c+bSO9GBWAp6lHIwfkH0tGVpZVVKTubfDIBdJvyu73xvCBKRc0EFeAF8vkuZDyjyKv6WKYZ6G0ydYsfvL4TysIGFgWDRl79V0sGRTg0/Xoix94pWc/WqKlzDa+WXM7vNRKrOc/L2avfTolWNjfiPFHqQbJ/yZ1h9NQF2bBYAdafYWzRd4KzYageJj/lGXQGE+Wj3Xt5A0pQWgcneP35ONmHLUjcjwg6iS9NG3HhMoN3SNe6tmVIZ/ke4/7UoB8TeovaDhZgyFBIHNBu5DQODQHHxKMxFBSQ5/sP+JBmrc50LKYXyjZIrQq8SbmxJTVpbN6UUVm6KBingvwW0FSo0692GsfcZ33WPc5mvULO9WMruWD2tftTmjHqyYzKmIJ9RjR9ajSIWbO0y6Q38TLmaHC5fQMhqaCt6LCJjl8MS6oQRs34/dF9Xt5FJyzFyFHxChf8qjPBOgahrjylrqZ9pccCbHdXpdF+1OMc+xDDCU0CvB0DeNmsgfOBWMDg/+bvKZTK AhhdQJNu A0qzlttw2QUDNmluhvfXUAF8s1T7QKt9b9lnvJPnHhr2/y+OuQWojKOA8yglMGfpfQ0wrYWE5YiMhWrxV8k5R8Ap/p32gzmfG58g7hDFLRjkK3xPEPBn36WQvsTtBZHHnm7ioghoHP2qBDUPaacWeSRy/hZnkwDDiLv90oP6k+2XWpoOKLDH9o44t36ys/9SNH/BFsTn2K2yx674DkD01c8V+ZgQIMpSu/vXyCtsYJhxZ5mJgDGw1Ek3SzS0v9qAtqVTrSOkZm7G+CNOU+jOJOwvwHWBVotOKGOcA65Fg7DlT8PJB5UXiZo5gO2DGVK23Vt0ASRSnFTavbG8vuxnrp+arTePD36kSpaNC 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 Mon, Sep 09, 2024 at 09:29:56AM +0800, Feng Tang wrote: > For current krealloc(), one problem is its caller doesn't know what's > the actual request size, say the object is 64 bytes kmalloc one, but > the original caller may only requested 48 bytes. And when krealloc() > shrinks or grows in the same object, or allocate a new bigger object, > it lacks this 'original size' information to do accurate data preserving > or zeroing (when __GFP_ZERO is set). > > And when some slub debug option is enabled, kmalloc caches do have this > 'orig_size' feature. So utilize it to do more accurate data handling, > as well as enforce the kmalloc-redzone sanity check. > > The krealloc() related code is moved from slab_common.c to slub.c for > more efficient function calling. I think it would be good to do this in a separate commit, for a better diff and history. > > Suggested-by: Vlastimil Babka > Signed-off-by: Feng Tang > --- > mm/slab_common.c | 84 ------------------------------------- > mm/slub.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 106 insertions(+), 84 deletions(-) > > diff --git a/mm/slab_common.c b/mm/slab_common.c > index ad438ba62485..e59942fb7970 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -1297,90 +1297,6 @@ module_init(slab_proc_init); > > #endif /* CONFIG_SLUB_DEBUG */ > > -static __always_inline __realloc_size(2) void * > -__do_krealloc(const void *p, size_t new_size, gfp_t flags) > -{ > - void *ret; > - size_t ks; > - > - /* Check for double-free before calling ksize. */ > - if (likely(!ZERO_OR_NULL_PTR(p))) { > - if (!kasan_check_byte(p)) > - return NULL; > - ks = ksize(p); > - } else > - ks = 0; > - > - /* If the object still fits, repoison it precisely. */ > - if (ks >= new_size) { > - /* Zero out spare memory. */ > - if (want_init_on_alloc(flags)) { > - kasan_disable_current(); > - memset((void *)p + new_size, 0, ks - new_size); > - kasan_enable_current(); > - } > - > - p = kasan_krealloc((void *)p, new_size, flags); > - return (void *)p; > - } > - > - ret = kmalloc_node_track_caller_noprof(new_size, flags, NUMA_NO_NODE, _RET_IP_); > - if (ret && p) { > - /* Disable KASAN checks as the object's redzone is accessed. */ > - kasan_disable_current(); > - memcpy(ret, kasan_reset_tag(p), ks); > - kasan_enable_current(); > - } > - > - return ret; > -} > - > -/** > - * krealloc - reallocate memory. The contents will remain unchanged. > - * @p: object to reallocate memory for. > - * @new_size: how many bytes of memory are required. > - * @flags: the type of memory to allocate. > - * > - * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_size > - * is 0 and @p is not a %NULL pointer, the object pointed to is freed. > - * > - * If __GFP_ZERO logic is requested, callers must ensure that, starting with the > - * initial memory allocation, every subsequent call to this API for the same > - * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that > - * __GFP_ZERO is not fully honored by this API. > - * > - * This is the case, since krealloc() only knows about the bucket size of an > - * allocation (but not the exact size it was allocated with) and hence > - * implements the following semantics for shrinking and growing buffers with > - * __GFP_ZERO. > - * > - * new bucket > - * 0 size size > - * |--------|----------------| > - * | keep | zero | > - * > - * In any case, the contents of the object pointed to are preserved up to the > - * lesser of the new and old sizes. > - * > - * Return: pointer to the allocated memory or %NULL in case of error > - */ > -void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags) > -{ > - void *ret; > - > - if (unlikely(!new_size)) { > - kfree(p); > - return ZERO_SIZE_PTR; > - } > - > - ret = __do_krealloc(p, new_size, flags); > - if (ret && kasan_reset_tag(p) != kasan_reset_tag(ret)) > - kfree(p); > - > - return ret; > -} > -EXPORT_SYMBOL(krealloc_noprof); > - > /** > * kfree_sensitive - Clear sensitive information in memory before freeing > * @p: object to free memory of > diff --git a/mm/slub.c b/mm/slub.c > index 4cb3822dba08..d4c938dfb89e 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -4709,6 +4709,112 @@ void kfree(const void *object) > } > EXPORT_SYMBOL(kfree); > > +static __always_inline __realloc_size(2) void * > +__do_krealloc(const void *p, size_t new_size, gfp_t flags) > +{ > + void *ret; > + size_t ks; > + int orig_size = 0; > + struct kmem_cache *s; > + > + /* Check for double-free before calling ksize. */ > + if (likely(!ZERO_OR_NULL_PTR(p))) { > + if (!kasan_check_byte(p)) > + return NULL; > + > + s = virt_to_cache(p); > + orig_size = get_orig_size(s, (void *)p); > + ks = s->object_size; > + } else > + ks = 0; > + > + /* If the object doesn't fit, allocate a bigger one */ > + if (new_size > ks) > + goto alloc_new; > + > + /* Zero out spare memory. */ > + if (want_init_on_alloc(flags)) { > + kasan_disable_current(); > + if (orig_size < new_size) > + memset((void *)p + orig_size, 0, new_size - orig_size); > + else > + memset((void *)p + new_size, 0, ks - new_size); > + kasan_enable_current(); > + } > + > + if (slub_debug_orig_size(s) && !is_kfence_address(p)) { > + set_orig_size(s, (void *)p, new_size); > + if (s->flags & SLAB_RED_ZONE && new_size < ks) > + memset_no_sanitize_memory((void *)p + new_size, > + SLUB_RED_ACTIVE, ks - new_size); > + } > + > + p = kasan_krealloc((void *)p, new_size, flags); > + return (void *)p; > + > +alloc_new: > + ret = kmalloc_node_track_caller_noprof(new_size, flags, NUMA_NO_NODE, _RET_IP_); > + if (ret && p) { > + /* Disable KASAN checks as the object's redzone is accessed. */ > + kasan_disable_current(); > + if (orig_size) > + memcpy(ret, kasan_reset_tag(p), orig_size); > + kasan_enable_current(); > + } > + > + return ret; > +} > + > +/** > + * krealloc - reallocate memory. The contents will remain unchanged. > + * @p: object to reallocate memory for. > + * @new_size: how many bytes of memory are required. > + * @flags: the type of memory to allocate. > + * > + * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_size > + * is 0 and @p is not a %NULL pointer, the object pointed to is freed. > + * > + * If __GFP_ZERO logic is requested, callers must ensure that, starting with the > + * initial memory allocation, every subsequent call to this API for the same > + * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that > + * __GFP_ZERO is not fully honored by this API. > + * > + * When slub_debug_orig_size() is off, since krealloc() only knows about the I think you want to remove ' since ' here. > + * bucket size of an allocation (but not the exact size it was allocated with) > + * and hence implements the following semantics for shrinking and growing > + * buffers with __GFP_ZERO. > + * > + * new bucket > + * 0 size size > + * |--------|----------------| > + * | keep | zero | > + * > + * Otherwize, the original allocation size 'orig_size' could be used to Typo in 'otherwise'. > + * precisely clear the requested size, and the new size will also be stored as > + * the new 'orig_size'. > + * > + * In any case, the contents of the object pointed to are preserved up to the > + * lesser of the new and old sizes. > + * > + * Return: pointer to the allocated memory or %NULL in case of error > + */ > +void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags) > +{ > + void *ret; > + > + if (unlikely(!new_size)) { > + kfree(p); > + return ZERO_SIZE_PTR; > + } > + > + ret = __do_krealloc(p, new_size, flags); > + if (ret && kasan_reset_tag(p) != kasan_reset_tag(ret)) > + kfree(p); > + > + return ret; > +} > +EXPORT_SYMBOL(krealloc_noprof); > + > struct detached_freelist { > struct slab *slab; > void *tail; > -- > 2.34.1 >