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 2B4E8EC8758 for ; Thu, 7 Sep 2023 19:43:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 446E48D0003; Thu, 7 Sep 2023 15:43:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3CE428D0001; Thu, 7 Sep 2023 15:43:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26F3F8D0003; Thu, 7 Sep 2023 15:43:34 -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 117818D0001 for ; Thu, 7 Sep 2023 15:43:34 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D7E1C4013D for ; Thu, 7 Sep 2023 19:43:33 +0000 (UTC) X-FDA: 81210825906.06.C3B4D07 Received: from mail-pf1-f178.google.com (mail-pf1-f178.google.com [209.85.210.178]) by imf23.hostedemail.com (Postfix) with ESMTP id 0C98D14001B for ; Thu, 7 Sep 2023 19:43:31 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=fic5ZrnH; spf=pass (imf23.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.178 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694115812; 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=JyxUbnx4HZmUSdrV+kf8fnRGR8ocSxgjnGxLCSFgGG4=; b=wS2faROIsPTv0qCVZEPS/e/SH8nFM3zPcZLlrUNYsrhojYHWjp5P+YYYUxU7anGt6qtP7y 2OKG6RRh8GJ9lUddkyh2MUcjXAdhKJ4SI9bnJjgI4PK8W3Y24eyCl2tZI972aeDS/eYj0F ZouW2lw/CM5dcYn4UIc3NGQsLAXBLvg= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=fic5ZrnH; spf=pass (imf23.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.178 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694115812; a=rsa-sha256; cv=none; b=Y2fIkkxpE2YFJBfzwYAEl+wE2Hx0WwT9qOaMoZ6ZYZVatpCxKd7nB6H6KByxM5X4yV/gmT 7TjmZr6qMfStMkrvqh4i9u7xH3lueX/Z4z+1HON2PXwvG9IC/cvEP1wCFoyeYBmbrWBW9N dlhhr/ZnCwqhl+SCSTmJud/0hEDNS+c= Received: by mail-pf1-f178.google.com with SMTP id d2e1a72fcca58-68e4700c931so483696b3a.3 for ; Thu, 07 Sep 2023 12:43:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1694115811; x=1694720611; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=JyxUbnx4HZmUSdrV+kf8fnRGR8ocSxgjnGxLCSFgGG4=; b=fic5ZrnHfwzVBFb42GbYu77zMXaP9pY+QT79/YoBtTSl0HYS6W+YKdEm+y3BLBLhnd Hb/LjJmUgpvL3pMmY9UdaPBc4WtdKB5fPsw0bjprqtOf3VDnoaDhqLy81iTwiloxPQjq 7kpi/o478KzS8E9VMe5AFZRdMeI9iVrG4QYGc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694115811; x=1694720611; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=JyxUbnx4HZmUSdrV+kf8fnRGR8ocSxgjnGxLCSFgGG4=; b=t7X7APXiW2qBOrOMRrDuOgCSVuRVLMYaPDjIVqUKzmVIWvPUtee1YGx1S/ZMhATDSV aeAF30aG9675ZiPO/VhEVBR7/nKldbHktqQ94Zb1Y87KVOjsx5YUgrRmsaUPC0ikC5S8 VAw6uYtow/PuMRHf2wgcEp1UeFGaeFhnccaHuRo+a75kUENYZqozHuzwl6BTgwKb/X2Z 13pOboWvedOB0v+TNag1kdZqsigBv6h9t0ldSgtpmcqNv9LGy77tAJEwgLFpQCPAyNha Q09URPVL62LKa6uEYxsNXLI3p53gQOTMI+SRELGcQoA2S+tDsvC1fE7dNyAMJcc38nty LlDw== X-Gm-Message-State: AOJu0Yzx+XEiEKJlX3zdhsUvZ+6pLj7K40BcsYK1KrhytCQc2aKxVsfN 4l+Qgn+8Zd737c1b2qrG4iWtc3Rj0GFTcxmyD7M= X-Google-Smtp-Source: AGHT+IG5Jr1XW8POkMawPbxQZk4DARRoztm3yclyrJHKWW2Ruzzn1M+lR/MFOnQpnMuPN1kSEcjZ0w== X-Received: by 2002:a17:903:2446:b0:1c3:7ed2:b54e with SMTP id l6-20020a170903244600b001c37ed2b54emr453742pls.57.1694115502592; Thu, 07 Sep 2023 12:38:22 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id v10-20020a1709029a0a00b001b2069072ccsm131820plp.18.2023.09.07.12.38.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Sep 2023 12:38:21 -0700 (PDT) Date: Thu, 7 Sep 2023 12:38:21 -0700 From: Kees Cook To: David Laight Cc: 'Vlastimil Babka' , "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 Subject: Re: Subject: [PATCH v2] slab: kmalloc_size_roundup() must not return 0 for non-zero size Message-ID: <202309071235.CB4F6B2@keescook> References: <4d31a2bf7eb544749023cf491c0eccc8@AcuMS.aculab.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4d31a2bf7eb544749023cf491c0eccc8@AcuMS.aculab.com> X-Rspamd-Queue-Id: 0C98D14001B X-Rspam-User: X-Stat-Signature: ow939f7i1rhuezhty5ez5k6bx4ffxfui X-Rspamd-Server: rspam01 X-HE-Tag: 1694115811-555657 X-HE-Meta: U2FsdGVkX1/KvS5CO6XhaRxe+/sG5X4QkW2QPmO3obFz/XY20c2pXXaTSduQnlL4R4LnN6RmmdEsI2MdQaTAXZRwDAvCGZO2+FfkzhEF59wHhkIYty/rZcsZGPA0Ge3W6SREfW+AKayJ5c3jW0p5CssFsVMOvx+WGdu6x10RmVWJayBbQhugi6GJ9oLFsgN8FHikxwntBXSjlgYYZXo/T9ZhDilCBJ+FARFR6iPRnDLjYiF4u69OqjetK25+rOpqmmRLZU3FsaGrC+ODUxirWcGDIQonVXjFdW/FcH6/4ME0m15V5UK6Iuz3LjaSRdsEFEPfCd3QOcdYrmNa2RBzUuPnoaz0uLB3nV+9pbyR3EFg+1upWluQemrvgz+QTvquFAsqq0OBGN4jyMQ2Iww7+75Foev9JE5/KTKjU9eCyv7comGq//NyjePgqK4QIKhSqFUZF9NpiSdm7bFvgueLRKA36G74aMRI2LKJCrfGrXu/lYAzQi68fOgqQY9X3PrkwDynOvKWGIPxs1rczVv5O2yMD4cy3pkNBptNdw6vixAgnlKTlcxYWUDj2TyFMbV9sPFzKdRRN1G+jUxkuFRD1dawhdjQsdF2ENwfOoZxhZxUcKe2WjCmGz9PHlkHP1W51aJDVOuIpQci6fsNMx0xAJRcRyu3a8wu4BUS8i18LOl8ubru3gcNt65a7XWjAm7/vCo+jrjuQRXEaYh9XP8AFlcfBzhfCWo9u3MBaMCVjvTe1q423Kl12PafIpkDsDlc9QbPdn6tQULPPprICmddt4yqCHmAOFJJ3QF903f2BhISRj0kBYM/d2/qjUyF8MIcYgKhLjwI3uWKJr/x+XfMDn7QxdiRzvow0u1uNKktiIaGMD5oXoOzBxkIA7UW+xZK6UllKXkS5hhswdlveP+C1/mShYU4q97ogD73/cDIcv4gcBTUYyvvEBUBugWViHTWFJaX51F5bhp3+lnfevS u45IgHaJ 5YQymzk2LwUOgN4MfZx5dnVtzClArAa6VO5rxZrCRSrGjtqgw0w/6FO1AaG+rvNnLtEckmpbzWlA993b17t+tqXOY3oQ6DiZcATeQoqjcE1o6kv41YDs9Z0xl1khoq549iQhL+y6xSZOt65mb2FwMByD0H/fkpyHRaFSG4iTNKAvhLLeY4T1z53TIOQODyEC9i1idVddAbmKGPm1oAF7iB0/v3PoMX+/NLrgQgwUoJH8Yaw+zuvWV0cyoFylhEWZb5acKCSDCy3P/2GWiJZkt2uPFfVxvVS8FAKVEM69JzmOonw6CUhEiXjeDZoOkMICRZrHAZpr+GI6zeT3wSQGczv0inRLNhTUAVwx8xx6AmdfFimOm9mNdDLTdwaJKy9lKTymB31wt7iermgCdeq508kukTWuhEBUFqDIh3f2ni4awfrb4H5e3/wodN/bTKsPB0Xo1sNW5gxpcFzGvvIxMDJWl/pwSzD+bM1YN 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 Thu, Sep 07, 2023 at 12:42:20PM +0000, David Laight wrote: > 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 > it's single zero-length buffer. > > Fix by returning the input size on error or if the size exceeds > a 'sanity' limit. > kmalloc() will then return NULL is the size really is too big. > > > Signed-off-by: David Laight > Fixes: 05a940656e1eb ("slab: Introduce kmalloc_size_roundup()") > --- > v2: > - Use KMALLOC_MAX_SIZE for upper limit. > (KMALLOC_MAX_SIZE + 1 may give better code on some archs!) > - Invert test for overlarge for consistency. > - Put a likely() on result of kmalloc_slab(). > > mm/slab_common.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/mm/slab_common.c b/mm/slab_common.c > index cd71f9581e67..0fb7c7e19bad 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -747,22 +747,22 @@ size_t kmalloc_size_roundup(size_t size) > { > struct kmem_cache *c; > > - /* Short-circuit the 0 size case. */ > - if (unlikely(size == 0)) > - return 0; If we want to allow 0, let's just leave this case as-is: the compiler will optimize it against the other tests. > - /* 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 likely(c) ? c->object_size : size; I would like to have this fail "safe". c should never be NULL here, so let's return "KMALLOC_MAX_SIZE + 1" to force failures. > + } > + > /* 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. > - */ > - c = kmalloc_slab(size, GFP_KERNEL, 0); > - return c ? c->object_size : 0; > + /* Return 'size' for 0 and very large - kmalloc() may fail. */ I want to _be certain_ failure happens. If we get here we need to return "KMALLOC_MAX_SIZE + 1" -Kees > + return size; > + > } > EXPORT_SYMBOL(kmalloc_size_roundup); > > -- > 2.17.1 > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > -- Kees Cook