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 438D6CE79AF for ; Wed, 20 Sep 2023 09:58:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9FCB26B013F; Wed, 20 Sep 2023 05:58:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9AC536B0140; Wed, 20 Sep 2023 05:58:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 84F5C6B0141; Wed, 20 Sep 2023 05:58:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 737516B013F for ; Wed, 20 Sep 2023 05:58:18 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 450651CAD72 for ; Wed, 20 Sep 2023 09:58:18 +0000 (UTC) X-FDA: 81256525476.10.E91B275 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf24.hostedemail.com (Postfix) with ESMTP id 1F88B180006 for ; Wed, 20 Sep 2023 09:58:15 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=MR3L8ZSG; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=lKFd2Oq5; spf=pass (imf24.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695203896; 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=C7etg/EV2WUJEwG7wK9oO1aQjQJcvB5sBjEfwzI17Yw=; b=5rUk98pWbz5q0STuiV1aQeTBIcI3TMX/DlgmrBGaqL+tCx7besm0XI7TR0zFt1ZQi0i6IE Qz3M7JX4BNTifr5WYc/2oJ6hzotiicUpSxIr0Zi6/rEt/zqKoBxkEf3/Zq4oUN9H0efBPZ KnD65p6PwYn+h8J/yW36m8tU/ak/AIU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695203896; a=rsa-sha256; cv=none; b=OK0W0bLkperb7lAwmod6C/G0TM23/B4NsEjWokCZfdgnWJfoB5YNo2iyIsxhA3Svrsbl80 z3GNZND9C6r3uSGUxTSOSuMxsPHy1Ao2ICTbFntgDosFDef6M+NFbwna7+Sa5RS9j3FRX5 9IpseCnWc51pufvQnJQuEk5j6OzY98Q= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=MR3L8ZSG; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=lKFd2Oq5; spf=pass (imf24.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none 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-out1.suse.de (Postfix) with ESMTPS id 63F492212A; Wed, 20 Sep 2023 09:58:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1695203894; 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=C7etg/EV2WUJEwG7wK9oO1aQjQJcvB5sBjEfwzI17Yw=; b=MR3L8ZSGioj7/m7NYCndkX9AGXys7GHVywviOzZM5a5LphNVXQ1teZMYKlz/w04kvFM601 15ZHupLaiynIlKNq/itEKLGkpswIDQUWMuk3gY5YWLB3SaDlvTUcqNScl+rbI2p4ABI8Dp 1MPWZgdaHQpCOrYNEN0v21SO5vXrlHQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1695203894; 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=C7etg/EV2WUJEwG7wK9oO1aQjQJcvB5sBjEfwzI17Yw=; b=lKFd2Oq5A4hZPiuD4O26oaYs7nJ4/7Y92ui8qz7Z4BkeNYRmo0RP/JUpxCslLVhfVVDur3 OdzYhgAn86I6dvAw== 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 35F0E1333E; Wed, 20 Sep 2023 09:58:14 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id FgB0DDbCCmUXCAAAMHmgww (envelope-from ); Wed, 20 Sep 2023 09:58:14 +0000 Message-ID: <5569ec99-b441-f7f0-060b-168abc089b23@suse.cz> Date: Wed, 20 Sep 2023 11:58:13 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: Subject: [PATCH v2] slab: kmalloc_size_roundup() must not return 0 for non-zero size Content-Language: en-US To: David Laight , 'Kees Cook' Cc: "linux-kernel@vger.kernel.org" , "'linux-mm@kvack.org'" , 'Christoph Lameter' , 'Pekka Enberg' , 'David Rientjes' , 'Joonsoo Kim' , 'Andrew Morton' , 'Eric Dumazet' , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Roman Gushchin References: <4d31a2bf7eb544749023cf491c0eccc8@AcuMS.aculab.com> <202309071235.CB4F6B2@keescook> <20ca0a567a874052a1161e9be0870463@AcuMS.aculab.com> <1bf41b7c8d7c428c8bfb5504ec9f680e@AcuMS.aculab.com> <1d634412-c0e5-4c16-92a4-447bde684ad6@suse.cz> From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Stat-Signature: nc1g5scno7izuf67dmx67xq7isernzym X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 1F88B180006 X-Rspam-User: X-HE-Tag: 1695203895-245298 X-HE-Meta: U2FsdGVkX19cip/QKnHeysj6HIt57Od23gPorBCgwp2wxAyJ1m2mweVI0l17TSRsW0xS5P8+nGfEKDZitjtyQcpz3V4Cg+CbwALbsADXm8p0A9v0p7tfmws1EdX/agAhGDCouZiD7gzFeqzyXUVgA8Bu7FAwhu2/6EOLWbYOQ7wwr3Qn6rnlZbSqSCFAClvKnMf6CNu3mw7DjL6wn+b7OGI27eqaINp58RkZ5F2GyTbmRg0CtM0LPG+wUGyy/R81i6C92ir29hrB0NIQZrhYrXMiNUpXX4NxH8ww6X+mEvG6RFlSHVXbIFz6NPfsVMYxV74fNZCQboBOq0lJtYEG+sAPDtKc0s/FZFXQzCMDsOTKzeP7w4wxbcQm/mud2Uwjg3SpjJo3epMNAabHAGDFIlvaUg2yvWvya+iNMppTwc5y8WDrZAi+K0m9Uz0a8vrbPllxv6c5LjmMBuPNjANNMjJzjLjgCyw8t1HvJiRBUgi7YzL3pwe/wceyiKVYmNPZddZ+zxaxS/6NWiqnQ6dCPVbqvg0y9RaZrRdM9NtyWgzVob8NSs6brmvilg5dj3F8TMfWCvVtJ2Sy3bMGd+l1rKk4TrIsjap1v8uXdlk95TEASVsY0EWRMRu75lUhWLZTJMBIs/TB7F7xWxK1E13SITS0IjPw4xe2b3+OW4KyAfix97dwh9Fu90z12Sup4cf6Jz5vcrB/eca838mWTOgUTWkbWveI8EjEdKYQ/+C9nFinkfj7GvN7vSme/04Umq5PVdTchSFHjbsbU47eLL9ALJRRtkw5xrkX9M52oteAKRubaVIkQGSafk50kRmtyGOuCH999azSxMQ00o5D61OYp2qp4b0XQsPQK3A/1Hc/SUGYNvITtv8dxxPXgareSvmDjSnyIFFu+pFG1AB/eo3KxjJLOTBHfZmxbZB+Ho8Fz1bw99Zh8/i5pdpz3iW+Ua1WL6hITxSHINCjC6XE39o 2IJkVxPY 3v34rHC6OuBukfoSJlC7JRfa355ZLldu/eaJJ/ehhQ0R2Vsx3VgUJQiXUvIpV//HmHH4wKr9KzcffJNRGzQ20sS1gXThhWo7TmbS3iKo2qdTbEMnSse5UoJPOp+WebQTpkcNTR1Np5SX/P6sAaGhW19CcIHlwv4B8c6Ci3Us888+/EeagM1Gk57+crJ9YnPGRE6xG5pU4k1cZ02PWR9GzxmVUN/ENfzPOr8qUHlRnj60+LOJOijMGRukH2hbt47m1W6ITP5LTejLHmR1Vt5Io3EsahhA9PymEhLUjMOz1RHCg1NJhdE3kE+lRpkbrP+c9wf/ZhRKzQY25CzUxR+18mcV8PwX0x0w8nDAY89Z7nsrRzfw55zj2CGfXY6S9z3he2XCvMzYU/fQ326JWU71fccMSSpO0tGN6nFho 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 9/11/23 18:38, David Laight wrote: >> >> So perhaps the best would be to return size for c == NULL, but 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_roundup >> probably could accept that too. > > It's probably just worth removing the c == 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? 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 = kmalloc(sz = 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 is too big. kmalloc_slab() should not normally return NULL, unless called too early. 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 crash during boot and the developer noticing an issue in their code. [vbabka@suse.cz: remove kmalloc_slab() result check, tweak comments and 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 == 0)) - return 0; - /* Short-circuit saturated "too-large" case. */ - if (unlikely(size == SIZE_MAX)) - return SIZE_MAX; + if (size && size <= KMALLOC_MAX_CACHE_SIZE) { + /* + * The flags don't matter since size_index is common to all. + * Neither does the caller for just getting ->object_size. + */ + c = 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 <= 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 = kmalloc_slab(size, GFP_KERNEL, 0); - return c ? c->object_size : 0; + return size; + } EXPORT_SYMBOL(kmalloc_size_roundup); -- 2.42.0