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 DCE9FE77371 for ; Sat, 30 Sep 2023 13:53:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 310B08D000B; Sat, 30 Sep 2023 09:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 298D28D0006; Sat, 30 Sep 2023 09:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 13AEB8D000B; Sat, 30 Sep 2023 09:53:35 -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 F1B078D0006 for ; Sat, 30 Sep 2023 09:53:34 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 9C09F160140 for ; Sat, 30 Sep 2023 13:53:34 +0000 (UTC) X-FDA: 81293406348.23.0BB72A7 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf15.hostedemail.com (Postfix) with ESMTP id D9904A0017 for ; Sat, 30 Sep 2023 13:53:32 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=L4YZd+rD; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf15.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696082012; 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=uE44gllyMSKTL2/oh+C5sUr05vfExpsU9yzwXpUaPDg=; b=g7QPy5iq+IE7gGogKubs+O9G1S4b94H7bJ3B8ADIXpA6UViGhsT8sZwH6T18WXbiCYg50S BhXBlO8l+/s8R/JJZBvaknf1U+2vsc4DQdKGe/wI1baBYDOm0dJOruGBxL9HVrBv8RJ9vd nkafn9wGcL0cyts2O8j6OM61mhn4sYc= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=L4YZd+rD; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf15.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696082012; a=rsa-sha256; cv=none; b=do8plqIaKN33qF/Bv4MVr3k2ttoc8FQdIaIEQfwUuT4098NNzO05OpWY8ofu0B95sx+jV9 F2l+yC59nAHJylSno00U2QcRhLdMAj5qHY7AMTRs/YfxUvN/P1Gw/yKCal9m8ZBfn2rBGC 8GgkXRDiHyPRKg5M4bIyb032Apwp+JA= Received: by mail-vk1-f171.google.com with SMTP id 71dfb90a1353d-4963adb481dso5720488e0c.1 for ; Sat, 30 Sep 2023 06:53:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696082012; x=1696686812; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=uE44gllyMSKTL2/oh+C5sUr05vfExpsU9yzwXpUaPDg=; b=L4YZd+rDR3cR5fmPqqCIpQo82q1v2YPR9ZJfnVtf7N67dFhS8QRLDOr2gYaWQ9CqFn 1Ux65YTP3USUmLEQsL+suKRSzW89rjhgX2sC4nBlu/eZ5x+GtjCISeSe+bpGV8ZCu7Bv PjwBee0KUyq8nCZovKbrxMD87F1QjD8VGVh/RHqbxPrr1Zn9TIR+zNc7yO8j1OWCz1qs oBjAL//II21QAP8I161b5oBDLLxftvUx1MAOjyXnQu+Yu5iSkmDnvrG0Q7kKdkDojuaw o1llWIvf+3zYLCYBaxRmIN+GYrS3AgweUK1e4QzKDNxKV6aW0Q/GveLihAMTLKZYNRGU jo9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696082012; x=1696686812; h=content-transfer-encoding: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=uE44gllyMSKTL2/oh+C5sUr05vfExpsU9yzwXpUaPDg=; b=UjtTE/Pw1qay/izLL4KGtpz6p0LKBmxZQoT6wukpPEip6Hzu8ty2Cq+wkALnsYhMLQ mDptdbR7ONMsZAboetgjrn8BHCUQgp4OojzqCsYcQhKI4ONNkYXW1R7AI1MaOGqaai+F +sHFmXrL3JxMpHdah13ZWST4pRA6Zfd0llVWxR+dNImzC1Ik9lSOfZC4MGIIiXhylr/s s8WkkEGxtM9jSfVgNMG75Dgp+eAkni9NBar8gR94bdrWLmg7zTRD2H1ervLNUfSaJfDD Fxnjx5rSgWW6hOJOQnXcW3o/LRY1v9y+J4KHYWSdH0uLCgj9d0zaHSym92dPwivfx3b4 OmfQ== X-Gm-Message-State: AOJu0YwdGTVHk82LK452pRL+pP6JSRJql8ioNmn9Hk5oNNcpXI9PF4C3 k54990n9WeDoqak02HOrcYSfeB7gXsVRmTk/eKA= X-Google-Smtp-Source: AGHT+IFgF9luMMCdL8FDm0tPqnvyIUnoBHwXCDg73udcbnOe9TI+UnqcrKovU9VnWkkpiGQqG8bMpRPqTY6PfyY8Fag= X-Received: by 2002:a1f:e043:0:b0:48d:1b20:268e with SMTP id x64-20020a1fe043000000b0048d1b20268emr5942014vkg.10.1696082011676; Sat, 30 Sep 2023 06:53:31 -0700 (PDT) MIME-Version: 1.0 References: <4d31a2bf7eb544749023cf491c0eccc8@AcuMS.aculab.com> <202309071235.CB4F6B2@keescook> <20ca0a567a874052a1161e9be0870463@AcuMS.aculab.com> <1bf41b7c8d7c428c8bfb5504ec9f680e@AcuMS.aculab.com> <1d634412-c0e5-4c16-92a4-447bde684ad6@suse.cz> <5569ec99-b441-f7f0-060b-168abc089b23@suse.cz> In-Reply-To: From: Hyeonggon Yoo <42.hyeyoo@gmail.com> Date: Sat, 30 Sep 2023 22:53:19 +0900 Message-ID: Subject: Re: Subject: [PATCH v2] slab: kmalloc_size_roundup() must not return 0 for non-zero size To: Vlastimil Babka Cc: David Laight , Kees Cook , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Eric Dumazet , Roman Gushchin Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D9904A0017 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: hp9frftzyw95zkkwugcoth8aqrqwsgxj X-HE-Tag: 1696082012-819381 X-HE-Meta: U2FsdGVkX196E61Ubbsr2DiPvdNBMhMiE9SyUI5HIX/GFWLJg05e/fzMlLpARhH2dc4B8M8ijz2PNagQnjv29s43CDXdr/pIT8BaKJinkvTpH1paIuYqLSRrpg4wyQbrS4y77y9+iOH0dSkD2rQt52v7z6mVMX+Qw6lY6WHECmav1DEXw+ixs6mnU5HXG/m2vSYQK3cbPlC6AsJsoBXKOOWugb4xiz7kAu+nEiBzB04sJQr7W/XUI1UsY8hqZC9+u4IvT3vx8EqO3i3cUZItDcNOdvlgH06jDym1GD+WPe+u6m5mrOZY1QkMIV5omcKOoxd1XBTXi3Z+lvKECOYePFny8EQWHMC4R01ESyFuJpQ68NDdzouKZblW+yuzkJJzGzRgqKtBv3a3b96w/y7Y74P10oZmydaqxMk9ef/h+VqLaRZ944cLqWNp+nRG4x99HEQDcOC2TnDjKB3mSolojc1GtOCjdhKFxrIy8r6HjBYFUcMzjOtUsz+ErAXmnjmB29DowB1355QwXtORpMd1Hlp9FLwdhEshF1vUZG56psBNBKBwK8nhXvIDw8Xe3LGkZfReo3R0biY0T7WZUQ11HyxA7WDU4RsrUpDypJr+yq7UPQplHcP4v1iE8EDhrM4W/rUYCR+uoKTdRQlUkJZzHj42kCgslUgpORraMvt2c8rMycypkCWeJDkVsD7qqKcV90CoDm//Bz4zgEWrvglqTYmcKxcGw3OJPT34ne+KknmVQlmkfDheOAAhArl/GR2TdCZ3ijp5JLFvHEcaLAEEfzdxqo/D39laWn7UmqdBN+Dl7IQYfZYaYFDoCePgvdasZ0ccpCEqtOzuM/0zUEvZXBJ3OWpf7lMdlGOL+nk+5BF5num53CRqOgC8zFc1cGMw9/3UAd4F/rrnrVlP/j4dhoLSEYvBgLRt+TASQ0wk47h0epqBJ4koJzbcZwxXU4q5aVAjqAgKsv9HOdu7Cjk OzSkCvqr Dy1cjeL4YBl7uYjg311154C/ueIa/7I1hWeikvOR54kCTN1cmxqJN3jnF5oZ6pdYIw+Ujf1vJI5TxpwvaeEsiAEcNGwF3NyO6TBf8vEzchobo8YoY/WQQsU06zbauRxN7DjBsmwxITAbQRdWek7v0zReycvyW3Ln5v8AqW3kW8iaidA5CAAsbH+jQJdMDAJQrzm++zwNoA3lhvRMPLoYueD0umbgpMvbbqvQz+W9JU1sB4nADw4IiFCZAqDH7c/Cd8c2SEed4kILB3UEJBF97jQ5S1NfaBaPSLIL6M79znSz7LuCs9Z445Wu4RDpcYVBpI+ajMUJU8yPjI6DrNcm0DkLVK8JjNMA/kTHXPTuVvwQ0jmLuKBo9LLDcphlK3VmVgBzC1xcT5WlyISOZwlL5GnU0vWNB5b15RGTr4Q98u40rslm/TQHTc++ZHounRsf06gQF+ei5ojbIiGtWyxzVQt1yOoAuEJ5QLbJk1enxnkS86IYf52lJLMSoGb883dZdAKDzNaqpN3MSfS2M4Cc959oma9UzCgLxDAVYGp5ellLmBthTUr1KR8jd3A== 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 Wed, Sep 20, 2023 at 9:48=E2=80=AFPM Vlastimil Babka wr= ote: > > On 9/20/23 12:06, David Laight wrote: > > From: Vlastimil Babka > >> Sent: 20 September 2023 10:58 > >> > >> On 9/11/23 18:38, David Laight wrote: > >> >> >> So perhaps the best would be to return size for c =3D=3D NULL, b= ut also do a > >> >> >> WARN_ONCE? > >> >> > > >> >> > That would add a real function call to an otherwise leaf function > >> >> > and almost certainly require the compiler create a stack frame. > >> >> > >> >> Hm I thought WARN is done by tripping on undefined instruction like= BUG > >> >> these days. Also any code that accepts the call to kmalloc_size_rou= ndup > >> >> probably could accept that too. > >> > > >> > It's probably just worth removing the c =3D=3D NULL check and > >> > assuming there won't be any fallout. > >> > The NULL pointer deref is an easy to debug as anything else. > >> > > >> > If it gets called in any early init code it'll soon show up. > >> > >> Good point, early crash should be ok. > >> So how about this with my tweaks, looks ok? > > > > Is that just/mainly the change to assume that kmalloc_slab() doesn't fa= il? > > Yes. > > > You can also remove 'c'. > > return kmalloc_slab(size, GFP_KERNEL, 0)->object_size; > > isn't too long. > > Right, did that and pushed to -next. Thanks! A bit late, but for the record: Looks good to me, Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > I also did a grep for callers. > > Nothing in early code, IIRC mainly 'net'. > > > > David > > > >> I'll put it in -next and > >> send with another hotfix to 6.6 next week. > >> ----8<---- > >> From f5de1ee7b35d7ab35c21c79dd13cea49fbe239b7 Mon Sep 17 00:00:00 2001 > >> From: David Laight > >> Date: Thu, 7 Sep 2023 12:42:20 +0000 > >> Subject: [PATCH] Subject: [PATCH v2] slab: kmalloc_size_roundup() must= not > >> return 0 for non-zero size > >> > >> The typical use of kmalloc_size_roundup() is: > >> > >> ptr =3D kmalloc(sz =3D kmalloc_size_roundup(size), ...); > >> if (!ptr) return -ENOMEM. > >> > >> This means it is vitally important that the returned value isn't less > >> than the argument even if the argument is insane. > >> In particular if kmalloc_slab() fails or the value is above > >> (MAX_ULONG - PAGE_SIZE) zero is returned and kmalloc() will return > >> its single zero-length buffer ZERO_SIZE_PTR. > >> > >> Fix this by returning the input size if the size exceeds > >> KMALLOC_MAX_SIZE. kmalloc() will then return NULL as the size really i= s > >> too big. > >> > >> kmalloc_slab() should not normally return NULL, unless called too earl= y. > >> Again, returning zero is not the correct action as it can be in some > >> usage scenarios stored to a variable and only later cause kmalloc() > >> return ZERO_SIZE_PTR and subsequent crashes on access. Instead we can > >> simply stop checking the kmalloc_slab() result completely, as calling > >> kmalloc_size_roundup() too early would then result in an immediate cra= sh > >> during boot and the developer noticing an issue in their code. > >> > >> [vbabka@suse.cz: remove kmalloc_slab() result check, tweak comments an= d > >> commit log] > >> Signed-off-by: David Laight > >> Fixes: 05a940656e1e ("slab: Introduce kmalloc_size_roundup()") > >> Signed-off-by: Vlastimil Babka > >> --- > >> mm/slab_common.c | 25 ++++++++++++++----------- > >> 1 file changed, 14 insertions(+), 11 deletions(-) > >> > >> diff --git a/mm/slab_common.c b/mm/slab_common.c > >> index e99e821065c3..1dc108224bd1 100644 > >> --- a/mm/slab_common.c > >> +++ b/mm/slab_common.c > >> @@ -747,22 +747,25 @@ size_t kmalloc_size_roundup(size_t size) > >> { > >> struct kmem_cache *c; > >> > >> - /* Short-circuit the 0 size case. */ > >> - if (unlikely(size =3D=3D 0)) > >> - return 0; > >> - /* Short-circuit saturated "too-large" case. */ > >> - if (unlikely(size =3D=3D SIZE_MAX)) > >> - return SIZE_MAX; > >> + if (size && size <=3D KMALLOC_MAX_CACHE_SIZE) { > >> + /* > >> + * The flags don't matter since size_index is common to a= ll. > >> + * Neither does the caller for just getting ->object_size= . > >> + */ > >> + c =3D kmalloc_slab(size, GFP_KERNEL, 0); > >> + return c->object_size; > >> + } > >> + > >> /* Above the smaller buckets, size is a multiple of page size. */ > >> - if (size > KMALLOC_MAX_CACHE_SIZE) > >> + if (size && size <=3D KMALLOC_MAX_SIZE) > >> return PAGE_SIZE << get_order(size); > >> > >> /* > >> - * The flags don't matter since size_index is common to all. > >> - * Neither does the caller for just getting ->object_size. > >> + * Return 'size' for 0 - kmalloc() returns ZERO_SIZE_PTR > >> + * and very large size - kmalloc() may fail. > >> */ > >> - c =3D kmalloc_slab(size, GFP_KERNEL, 0); > >> - return c ? c->object_size : 0; > >> + return size; > >> + > >> } > >> EXPORT_SYMBOL(kmalloc_size_roundup); > >> > >> -- > >> 2.42.0 > >> > >> > > > > - > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, M= K1 1PT, UK > > Registration No: 1397386 (Wales) >