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 EC938C3DA61 for ; Wed, 24 Jul 2024 10:12:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 422026B008C; Wed, 24 Jul 2024 06:12:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3D1696B0093; Wed, 24 Jul 2024 06:12:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 298E36B0096; Wed, 24 Jul 2024 06:12:02 -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 0A4656B008C for ; Wed, 24 Jul 2024 06:12:02 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id AA719A07A9 for ; Wed, 24 Jul 2024 10:12:01 +0000 (UTC) X-FDA: 82374230442.05.B11ECC4 Received: from mail-ua1-f45.google.com (mail-ua1-f45.google.com [209.85.222.45]) by imf12.hostedemail.com (Postfix) with ESMTP id D8FF240021 for ; Wed, 24 Jul 2024 10:11:59 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gZvNCFxt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.45 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721815881; a=rsa-sha256; cv=none; b=KeKgnkun9l/QX1ehIWbKu7pn7hXY1RC+aBY5AQipOSWahNRAzLT28IE6Dekfv5VcjQLemB VUaG3Im32emj3a3VCjccX+SwQ9DinByYLiwxYDEBrauloQYWqq25KPB+gk+S1IWAKTYRJD t7g9Xl0OFveKJTuOTFurK1IotFktzwI= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gZvNCFxt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf12.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.222.45 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721815881; 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=U9GRB8RAjw9CtuU//c8IdZMqiwuacvDKkdwwbVUCl0I=; b=gy2FIeUDxV5sIevC+6yeknq3XaG1pu9Mpj4bEVuDDyypI7wUnuooTjDEXsB3K68zkcQZkm Fkpan+wlAgsbsVrC8n6LqahUwi4qbbXeXKhdgZ2/iMhaPcUtfJ0SrUVKM6e5AQ2ifIAfdF 1yHEnFjtUlGfobuVEykUxkuSdQXLMSk= Received: by mail-ua1-f45.google.com with SMTP id a1e0cc1a2514c-821b8d887b8so2032831241.2 for ; Wed, 24 Jul 2024 03:11:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721815919; x=1722420719; 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=U9GRB8RAjw9CtuU//c8IdZMqiwuacvDKkdwwbVUCl0I=; b=gZvNCFxtUvNOOG++fTBGd5PyqqS9hkit9KuESsquH3hoefs3LV89j7QuXDZUD9qH5j PLVM5+VWx00DEcj+/aeUCKpthiNp6nuIYZH4YUl6f8eq3SbkqJ+rtu1apNpZtu8WVKtW gz+7bSfe46jCJxpb0RL77BHXUnYXt3WgViqJDxv4W0aznX7fwou+uOmaSuYeuF5/bM26 iEGbHzAcXD529tFsd8EOl4OMjyK7HU7uUq+Sr+7jmNqKsxBh5kwUYkKTWN1k9JUDHZj3 L4ew1FD0MA7dweICK+aCozyaV0a1zO0+TqM0Wvvrq6uIcBIMHkpeMETgw26XDH/yPb1N yyPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721815919; x=1722420719; 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=U9GRB8RAjw9CtuU//c8IdZMqiwuacvDKkdwwbVUCl0I=; b=OThUAxbCEOZqD/rF5JslIG3nLbLPuY0k83/Z57L7pwaKXmxSJft5v13u6ANpjpn3Ea k6gl9etrJ3BhQOe2zhQhE2ZQFw93Knf1Qcr2Ja60Pw9NAxltjlZ5hAEcN0RsJUc85pZG pKyu02VuhP7MPrfBdw+iS1Mj5c513KaSCNWqL2/5n0dN5haR4PSD2JnfdEQP551l2y3l Y+qU7OGa1n36GsPRiB+ZOChsOL0EHtYIT1E+iST4F1OafQFNh8qeR5JLI98TKeo58Wph Uu/z1XZJWxzIx2RAbRITgJLNxf6r2mkQfUtEFklz5Wa90//pvrX9vseWGFBqQu/rGjcV Uj2w== X-Forwarded-Encrypted: i=1; AJvYcCVoO3Q/D1nfoGjmFCFYULbF8aoBQmJeMf6cLkWcj2p3Ucb/15BLVZ4GQrj+XUraPDopG6nejb3CEUtJQN9AccGWCjU= X-Gm-Message-State: AOJu0Yw57/qzckeavjd6AWTIGuvmykbk27dYS2hsUF5WJdxmGrKjnjtE jfQV6CaocB7cxgw4Bzm438+/DNeLrMEDd7P4RYei3w35sakc6Dw12T0VPm20edRvx/T3sk4kQP3 SKDCUuJXz+Ybn1T6dZ1W/pAXOtDM= X-Google-Smtp-Source: AGHT+IFzE44rmuWnOKwwOuqrwpZBAHJmtmTqgX0cVzJxoiAsk2PFZBqXCXEHq1NI2cmdwGdjMCjST76ctlRVoCFj7Cc= X-Received: by 2002:a05:6102:33cb:b0:48f:e09e:ef60 with SMTP id ada2fe7eead31-493c4bc3bc2mr965783137.29.1721815918800; Wed, 24 Jul 2024 03:11:58 -0700 (PDT) MIME-Version: 1.0 References: <20240724085544.299090-1-21cnbao@gmail.com> <20240724085544.299090-4-21cnbao@gmail.com> <419e9b88-de76-4f17-a785-2579f62ad880@suse.cz> In-Reply-To: <419e9b88-de76-4f17-a785-2579f62ad880@suse.cz> From: Barry Song <21cnbao@gmail.com> Date: Wed, 24 Jul 2024 22:11:47 +1200 Message-ID: Subject: Re: [PATCH 3/5] mm: BUG_ON to avoid NULL deference while __GFP_NOFAIL fails To: Vlastimil Babka Cc: akpm@linux-foundation.org, linux-mm@kvack.org, 42.hyeyoo@gmail.com, cl@linux.com, hch@infradead.org, iamjoonsoo.kim@lge.com, lstoakes@gmail.com, mhocko@suse.com, penberg@kernel.org, rientjes@google.com, roman.gushchin@linux.dev, urezki@gmail.com, v-songbaohua@oppo.com, virtualization@lists.linux.dev, hailong.liu@oppo.com, torvalds@linux-foundation.org, Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: D8FF240021 X-Stat-Signature: gh1zbnfgzffsbfufts8cssinky1je5em X-Rspam-User: X-HE-Tag: 1721815919-125350 X-HE-Meta: U2FsdGVkX19gLWBFkc+DrXcsRY5bAkgSzDjNkc/V4Jm8kGR8zD+pFwsKCtvsJrA/TXUAJWowDrw03SWvrTbYo9bzWMGsZbva+TsnKr8RLKkxhelpkW60buEn8z3blr0RMrBWf/u5/ri1x3i2Bu6yaNdYB4tgRiw2yBgxmkzT7AWgO/Umkhs5ssTx4sSv14GTtXebJftMA81tmkiP0TUX07F1LjjWYnLXALyIfj92K24diuD2n3WPQtnBMu7R4syNgiUC0LDuD8K1b/7+/8w1xwkcH0TiDWqV3bL+VzVk5Oz/1WVH8a8NmvAU16ZkscJSSwL16H0SRnpCcrv4rVDOkUOJju2FIpVossXP5A8L8Ea5Glr9W9SqwyDBwPh0o2b657kL8vlfyVwPzNx/YgOgCPg+EZcW/3NPZZKOgZagyrWm9eYRNNQjvaevZzzrFjaog7EJjPAubAghfHnq2byfdxx5r8MFvm9yyJzXlAI0ULHKRBj+rn8Xe/hZBAsXmPh5oGhg1IDaXhvvWndMNSmEy+Yrr2O3eueFd3j7f8Ey8SkvTA/tlJ/SZPmTlKkZrmceWFi6ecmyD/4SNLwg+/99cGHjzqUgiP2QOC8H+s7HsTUzlAQhIxTXTAR9Xw+adtLmpVrCexfJ25+L0PPpMfzaEZNjYIY22yhbWiYqHiFhAmF0OsLWoYEmTEFEmaM9xDxtMFSoH3chbnJB8+AGMV97/jWtBhxfxsJFfqo1rl3+MEi7t9dZnmOf9W0H6YyeWQm/07PcGq0fcseXWbM+7qEgbffJn1BH8H8iAQyNSkdjV5XpHrzknx7dLiZHu+/xErc0UH0Utf+2GkWsZNyyMiqXV9cfS/Fu7fSHBjcF2WD7YL+mHFftCCJLaQLQA7bmnbchAlLX1XwL/jebdDCbh198/KOeya/z2uTm9i92A3QL4ivZfJ9PuBrt92rLkITYrpMppZ/hv8jD5VLP8XpuSmY fz2XOWhy QauwBnBEHCGrKAtRXvkRk6B9TybjT9Tpq5AYhw56R4PcyEtvUp9fQZE4Fz1HNXAMCw3/dozxJQqXfqnKA5jdVuUmP9aicy3J2SzGDMX9xBZeFvUgW6e0rVXfdCajUkZ/1IfUS4UiRmmm6rB53Cqq4IF8oiJTL+evn0+Gj8QHAXpKcuHqG3rk3rk6B4ewdznIBsnxchTUKCg8Ypq7vZ9VUaQN3XVnNLunwSRpFFMbb4LYRrHwna/p4niHV54AZQr4UVGd0q+1h3bo92HYinp4jYhh0qCpSal5m4O6rclhv99BsbfQy+izgWWEFbioOQ/cdtZ+pR9XpQLvhLJZRLh4UNz0tR0xISrtQ8DZm+Ne2j4Qa0d8gXLgzGvsnjr4CbnqvhIOpTZatdnlkkARDAPhse6soao1ZT46q2vZbfW3bRDNO3TQA/Z/tEZcqQ6mXcpctyuHi6itH8uYAOMB4YJphhStWS90vVcBIOw/V273RJ1Q2s7zu93KtxLf6q3bPS0D0gzNWUao2Zhj/21G2KhwsqacDe2BNVREhr2b44lmGfLRJjDKuytrvYW1VmGcaAIljhPdM/mI9utUFYMQGhKxFOBlP0kTlq2fgY+9hu7dp7g+9GLh2WC/IKDxtv4k29u2mcI3sx17ktmz8xYgg/LIfQ6P9OHNnLqefBxs7wZ7vAN28CEaWcxG0QrxvZfMIip6AiUldD44skx50IeU1U90dcJPGOb6m55VXICYcEUpxqXrn//32KLamRnY7eaB36rxlhqDcR473AKWsoPiKz0KDScnq8Q1MJSAzXaxl3dlWlrpS2KdqZhNoCC5BNDepj8yOi9pBehXqAHHJ0l6rAFdCD5p+W3Y/GnjTs09OMCp76a7cEzs4hH8dJF9KXg== 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 Wed, Jul 24, 2024 at 10:03=E2=80=AFPM Vlastimil Babka w= rote: > > On 7/24/24 10:55 AM, Barry Song wrote: > > From: Barry Song > > > > We have cases we still fail though callers might have __GFP_NOFAIL. > > Since they don't check the return, we are exposed to the security > > risks for NULL deference. > > > > Though BUG_ON() is not encouraged by Linus, this is an unrecoverable > > situation. > > > > Christoph Hellwig: > > The whole freaking point of __GFP_NOFAIL is that callers don't handle > > allocation failures. So in fact a straight BUG is the right thing > > here. > > > > Vlastimil Babka: > > It's just not a recoverable situation (WARN_ON is for recoverable > > situations). The caller cannot handle allocation failure and at the sam= e > > time asked for an impossible allocation. BUG_ON() is a guaranteed oops > > with stracktrace etc. We don't need to hope for the later NULL pointer > > dereference (which might if really unlucky happen from a different > > context where it's no longer obvious what lead to the allocation failin= g). > > Note that quote was meant specifically for the "too large" allocation, wh= ich > is truly impossible. That includes the kvmalloc_array() overflow, order > > MAX_ORDER etc. I equally quote this for two cases because non-sleepable is also returning = NULL, in this means, they are currently facing the same problems. > > The "can't sleep/reclaim" is a bit more nuanced as there's the alternativ= e > in just warning and looping and hoping kswapd or some other direct reclai= mer > saves the day. If yes, great, we have a system that still works and a > warning to repor. If no, there's still a warning, but later soft/hardlock= up > hits. These might be eventually worse than an immediate BUG_ON so it's no= t a > clear cut. At least I think these cases should be handled in two differen= t > patches and not together. But I fully agree these two can be separated and judged separately. After more thinking, I am concerned that this issue might be difficult to b= e rescued, as the misuse of GFP_ATOMIC | __GFP_NOFAIL typically occurs in atomic contexts with strict time requirements. Even if some other components release memory to satisfy the one busy-looping to obtain memory, it might already be too late? > > > Michal Hocko: > > Linus tends to be against adding new BUG() calls unless the failure is > > absolutely unrecoverable (e.g. corrupted data structures etc.). I am > > not sure how he would look at simply incorrect memory allocator usage t= o > > blow up the kernel. Now the argument could be made that those failures > > could cause subtle memory corruptions or even be exploitable which migh= t > > be a sufficient reason to stop them early. > > > > 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> > > Cc: Linus Torvalds > > Cc: Kees Cook > > Signed-off-by: Barry Song > > --- > > include/linux/slab.h | 4 +++- > > mm/page_alloc.c | 10 +++++----- > > mm/util.c | 1 + > > 3 files changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index c9cb42203183..4a4d1fdc2afe 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -827,8 +827,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))) { > > + BUG_ON(flags & __GFP_NOFAIL); > > return NULL; > > + } > > > > return kvmalloc_node_noprof(bytes, flags, node); > > } > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 45d2f41b4783..4d6af00fccd4 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -4435,11 +4435,11 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned= int order, > > */ > > if (gfp_mask & __GFP_NOFAIL) { > > /* > > - * All existing users of the __GFP_NOFAIL are blockable, = so warn > > - * of any new users that actually require GFP_NOWAIT > > + * All existing users of the __GFP_NOFAIL are blockable > > + * otherwise we introduce a busy loop with inside the pag= e > > + * allocator from non-sleepable contexts > > */ > > - if (WARN_ON_ONCE_GFP(!can_direct_reclaim, gfp_mask)) > > - goto fail; > > + BUG_ON(!can_direct_reclaim); > > > > /* > > * PF_MEMALLOC request from this context is rather bizarr= e > > @@ -4470,7 +4470,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned i= nt order, > > cond_resched(); > > goto retry; > > } > > -fail: > > + > > warn_alloc(gfp_mask, ac->nodemask, > > "page allocation failure: order:%u", order); > > got_pg: > > diff --git a/mm/util.c b/mm/util.c > > index 0ff5898cc6de..a1be50c243f1 100644 > > --- a/mm/util.c > > +++ b/mm/util.c > > @@ -668,6 +668,7 @@ void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(siz= e, b), gfp_t flags, int node) > > /* Don't even allow crazy sizes */ > > if (unlikely(size > INT_MAX)) { > > WARN_ON_ONCE(!(flags & __GFP_NOWARN)); > > + BUG_ON(flags & __GFP_NOFAIL); > > return NULL; > > } > > >