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 55A68C3DA60 for ; Thu, 18 Jul 2024 07:22:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C5CA16B0092; Thu, 18 Jul 2024 03:22:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C0C626B0093; Thu, 18 Jul 2024 03:22:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD4E76B0095; Thu, 18 Jul 2024 03:22:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 8D7C16B0092 for ; Thu, 18 Jul 2024 03:22:51 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 28AA714196F for ; Thu, 18 Jul 2024 07:22:51 +0000 (UTC) X-FDA: 82352031342.07.D106CC0 Received: from mail-ua1-f52.google.com (mail-ua1-f52.google.com [209.85.222.52]) by imf07.hostedemail.com (Postfix) with ESMTP id 5791740011 for ; Thu, 18 Jul 2024 07:22:49 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CbMdoDrs; spf=pass (imf07.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721287315; 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=xnAJ1b6tRP9UwaZPaWCm9nlM2G4UZ22W7qte5PpPuRw=; b=tyXoAK9JeVcb1viameuMAdchMb9xzg2IZC7mEtDIgzwnp0OpSSeeLt2N+aSOurLUMCNj/D k7f09HCjPBGRxDvckhpk0PXol1ZwuK++EtkaaGjxdN3OJQJf9Xn8nr/CWxDNz1ffzZ0Apj E66BqZBFpr1Z9fKQuOWDzKAE07bUkTw= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CbMdoDrs; spf=pass (imf07.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721287315; a=rsa-sha256; cv=none; b=AIyG8/7g/sxG1LN1lwlLV2g7cxivKpgQq8Cxpy9kKaegjmJSR7ErSfnTGg9vy2xKdOM9RB Qi63aZ3oMbWE2h7LbjgFYvWKv2eJLqKbJElDLk18lmLPmQrNklgTA80HLlxa6Y3Ym5HxLJ zmR/zzSZDbQrFaptLzhH1id6ELsXXkw= Received: by mail-ua1-f52.google.com with SMTP id a1e0cc1a2514c-81ff6a80cb2so189329241.3 for ; Thu, 18 Jul 2024 00:22:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721287368; x=1721892168; 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=xnAJ1b6tRP9UwaZPaWCm9nlM2G4UZ22W7qte5PpPuRw=; b=CbMdoDrsBDl9AciWE3/7PMRjJ7yLYCS4ch3u+V9X1abo6xAtEIWod9QJO0ENeF9YeF QA4PtWaOHC5wdYF/kmFpjOWYhCfD/h4/V3VsMHoMXz5J/BXmghUAemdJkDljtm8ofEQE kPQuSRHSRmZ5WMCSnx3qVNvU5pDEtpE2RripdLW5Yh8S/UtpTWwDP7G0XRYMQT195ryT MasLAG3otov6iXNvD8K5OCEq3NAD5xmLNeYQM7Vr1jpW9VNJUlk7bMX0hK2aSRcgf4EJ jCCzDzED9xGNk5yrX9vghYq36hlbQD69UllhsSq+jMDHJbZx0miKJl6DVxEUt2KvD81d rRQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721287368; x=1721892168; 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=xnAJ1b6tRP9UwaZPaWCm9nlM2G4UZ22W7qte5PpPuRw=; b=R6VoPsOQaw38Ns5t7SWrXsNXElYUB3mUc6T2lx6YSVXtK7tb5Vxj20ENSSH8LAmbXT AtaMsoBDhyISO75SY+Rf8HF+s5CeIjj5Ob0TPht1QWLxn8xQmUHUtEtX88QTgTsw3+b8 RRSGR355ShjYkYOhqna0QkdxRVmbHxD27JqXDan+ZAbglwWGexTXXohjltK1t3cesEqi rTgD5WrkKLE0FP2OZugBdUpfVn6tHWK2S4w3/QY5ctIShiY+ZAxPujR+c2aP4iUDjUGz 2C7EPZL05wov0d/D1LgGrNlr+uGnzHzC0pRfAeRhRaL7xso6bpabIrQF7oyXByereY7b EPDA== X-Forwarded-Encrypted: i=1; AJvYcCUirZlYYVEOR3SruFONB5wyPYW9tTrM6afSAOcGxFaksAaOrflozN6hJaqUrNgUxYsum1RY/0Eg0v1hPGOLeQVYqQM= X-Gm-Message-State: AOJu0YzsG4XThx8uEP53zsTGT8fOJzWwpi8KBle/h8TCkrX1BUvaPVzI yIzs/qpRqxfNM733DtYmeygpOtcV3GXhD2ewXMK03wNp2A7oROxCakTnIrqGLvC772/+Aw2QLGn sc/2HL6mAnufW+9FmgVbmdlVsxg4= X-Google-Smtp-Source: AGHT+IETI3jFFmXy8ZQsJQqfT+iAKYc93lJCXRhwz1O5d9DOgeFOyU7bDCRzIc2wU0uNE12npyMnGq5dAHGLFhgO8nw= X-Received: by 2002:a05:6102:5e93:b0:48f:4d5c:69cc with SMTP id ada2fe7eead31-491598e9ff5mr5359821137.11.1721287368261; Thu, 18 Jul 2024 00:22:48 -0700 (PDT) MIME-Version: 1.0 References: <20240717230025.77361-1-21cnbao@gmail.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Thu, 18 Jul 2024 19:22:37 +1200 Message-ID: Subject: Re: [PATCH RFC] mm: warn potential return NULL for kmalloc_array and kvmalloc_array with __GFP_NOFAIL To: Michal Hocko Cc: akpm@linux-foundation.org, linux-mm@kvack.org, Barry Song , Uladzislau Rezki , Christoph Hellwig , Lorenzo Stoakes , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 5791740011 X-Stat-Signature: oqyk5cos3r9g9ub1qx5j6ek4f365rzna X-Rspam-User: X-HE-Tag: 1721287369-639796 X-HE-Meta: U2FsdGVkX18O5hn6744llRzODrvHCUrb8LMccnjp57DupgxJm3ltxD/38V0kCrCPIkdYg9IipvxOXDAw9o757fMx7MfamG0edp2DEnv5YTmLoUZ4mdJ97np9+eS+lakE4hWyivkU+EbMO64iZsmJNEQQAegrlRQzGD6wb5bvAjT5/RfMtIx+KuaULaL8u40f0PLCr6sYiGpSNMLwhSU9GDpwdgJAAMyoPC89LSiK7udAgxuDmVOuPWnvOTXZSbKVj1fEDYVgnbOW/fxyL3lt2W+E7XEsOPILmYeeu5TGjksrB+xnPrXHwXimVi54fFq/AeH6sRHhuP7iyshSXtFB3lrZyExpEafdcD68R5zZOodqKNKWPrEnSBHnDdhIC99fGsTaOPblsZy4+UjT6sUsVCI1tmFfU/tvNjXmnpSNtHZlgRn86rzo54Go9PSmoVh1kFeb5gBTTd6uTEm5vjVbrC+wAl6ftG+FS1V5mndGS6F3ENS9VOpO6iDXrHmwzPlVvj8I5RJ/1GG2UUABuTjfaIqBGx2NPcH/HRWhHo/V+5XilMVAlmC5Irywa2sbYlkDr8Qz8YQa4lReXolM0dDBsYIf7D0be2GTKP2xPGA7q6MDHcRAMg2u5Kt3XEiVv/LGNTnQwa3C/qJ1eajKRGoKQqOAUSsx2cxW5cUMCvtKvRdrYX9EgcUjB8Cw63J/fXgWH4uULObvCyKe35bIzLsRr3kDyoHVSB3ueNIHRMCWeRsbNRTVMzmjDRRRU55MpaeO1YM4S1gIqP65nYzw10QJuXl9w4KnwD0EZUKQu1Y/TkXp4k5JJXux3tchzk+Xpa99PNJiq03gkTeulPWgcu6QKxrBx4Zi05Dygzz22WU6k1wYwhZdN2SrKq9zTdF23ecbrnRWcK1FlDAEz3Y1NpgGwulvVMy6tYC/5XDsLtsh8ELLswD4k5/gSdeyOSs3riFshTtW/O6R6o5ep1qREbK Yf0jVwJ+ 36gtpdr4aNqqvaMVnpg8k927aZKn8ST/eG2Qmc1xaqxDXqTrZDSKmIJZujv2Go23UD87NaukoPketdfKyMsY9i/3SHE0SkhdQVAohsDDlrH8l5lym0tNgU3fJMiejzySXDPJwbTFI5Gt7CBZxrgHA3XpEzq7WezoVnBDwzP/fEGaMsSisH2T7N9NRW5DXcetTpKDrq9HtqMGJKn6z5XUuv7Zq0acdkdKFN3RglO1N6q92czyqwTI/mhhgwBhaQGJ8CnhttDS0I1NFKW9sDbtuhntkE+NFwXQUH/wqKT7nLqM6Ahl5eq/jWqGAU+TBncIVugKL2yVIcX1T09xbL8bICAOLWPKkMmozElPqdd5CI1nBpUxDQoR9rU633oVqNN0/FnDEdO8RGBrxDenE5tuYxRUHK1dLZmgmuEqpG4yu0W65xzmmOEsMI/YkkIraHtegbAOZ/6hpPI3gobG0Uz+bwl8FbKWL0LxqjtH9ophh2wPFvHHQJTeqLCD/kmi+vzcZPFjmVgouVxaRI2L5hACEU4y0fdd7Gh4+SbiqbSKpy1JZ28rfl7uSc/132xsJccq2MgUxcBh3n4SC/q1R4k0jWUcflaNgmaR8v5W62UcUf9OhjQOZu/mz/HaALVS/f4Hqbepvw5rVri8PLpGKP4vTZEM943nv4AuXaKR/bMkdYsDpxoseFwqBa8EPEbFzjKNw1DMdVyjKx2QjUgpDPNt26cokEjQEE1tBcaXF6bCzHPApwvLu1Ma6Eu8TOqJ/tz82HEkAcFkTfTRsxUwsaH2xSRjysOOGFwR2NbfPTeWc2eqYeJUOxrH6AHihhUrbPNdtF1SC 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 Thu, Jul 18, 2024 at 6:58=E2=80=AFPM Michal Hocko wrot= e: > > On Thu 18-07-24 11:00:25, Barry Song wrote: > > From: Barry Song > > > > Overflow in this context is highly unlikely. However, allocations using > > GFP_NOFAIL are guaranteed to succeed, so checking the return value is > > unnecessary. One option to fix this is allowing memory allocation with > > an overflowed size, but it seems pointless. Let's at least issue a > > warning. Likely BUG_ON() seems better as anyway we can't fix it? > > WARN_ON is effectively BUG_ON with panic_on_warn so if this happens to > be in a user triggerable path then you would have an easy way to panic > the whole machine. It is likely true that the kernel could oops just > right after the failure but that could be recoverable at least. > > If anything I would just pr_warn with caller address or add dump_stack > to capture the full trace. That would give us the caller that needs > fixing without panicing the system with panic_on_warn. > > Btw. what has led you to create this patch? Have you encountered a > misbehaving caller? I didn't encounter any misbehaving callers in the in-tree code. I was debugging another bug potentially related to kvmalloc in my out-of-tree drivers, so I reviewed all the code and noticed the NULL return for GFP_NOFAIL. For future-proofing and security reasons, returning NULL for NOFAIL still seems incorrect as the callers won't check the ret. If any future or existing in-tree code has a potential bug which might be exploited by hackers, for example ptr =3D kvmalloc_array(NOFAIL); ptr->callback(); //ptr=3DNULL; callback could be a privilege escalation? I'm not a security expert, so hopefully others can provide some insights :-) > > Realistically speaking k*malloc(GFP_NOFAIL) with large values (still > far from overflow) would be very dangerous as well. So I am not really > sure this is buying us much if anything at all. > > > Cc: Michal Hocko > > Cc: Uladzislau Rezki (Sony) > > Cc: Christoph Hellwig > > Cc: Lorenzo Stoakes > > Cc: Christoph Lameter > > Cc: Pekka Enberg > > Cc: David Rientjes > > Cc: Joonsoo Kim > > Cc: Vlastimil Babka > > Cc: Roman Gushchin > > Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > Signed-off-by: Barry Song > > --- > > include/linux/slab.h | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index a332dd2fa6cd..c6aec311864f 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -692,8 +692,10 @@ static inline __alloc_size(1, 2) void *kmalloc_arr= ay_noprof(size_t n, size_t siz > > { > > size_t bytes; > > > > - if (unlikely(check_mul_overflow(n, size, &bytes))) > > + if (unlikely(check_mul_overflow(n, size, &bytes))) { > > + WARN_ON(flags & __GFP_NOFAIL); > > return NULL; > > + } > > if (__builtin_constant_p(n) && __builtin_constant_p(size)) > > return kmalloc_noprof(bytes, flags); > > return kmalloc_noprof(bytes, flags); > > @@ -794,8 +796,10 @@ kvmalloc_array_node_noprof(size_t n, size_t size, = gfp_t flags, int node) > > { > > size_t bytes; > > > > - if (unlikely(check_mul_overflow(n, size, &bytes))) > > + if (unlikely(check_mul_overflow(n, size, &bytes))) { > > + WARN_ON(flags & __GFP_NOFAIL); > > return NULL; > > + } > > > > return kvmalloc_node_noprof(bytes, flags, node); > > } > > -- > > 2.34.1 > > -- > Michal Hocko > SUSE Labs Thanks Barry