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 8D060C3DA61 for ; Wed, 24 Jul 2024 12:10:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E8DD66B0088; Wed, 24 Jul 2024 08:10:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E3DEF6B0089; Wed, 24 Jul 2024 08:10:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CDE446B008A; Wed, 24 Jul 2024 08:10:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id AEC236B0088 for ; Wed, 24 Jul 2024 08:10:34 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0B57640891 for ; Wed, 24 Jul 2024 12:10:34 +0000 (UTC) X-FDA: 82374529188.03.1E23443 Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf28.hostedemail.com (Postfix) with ESMTP id 1D8DBC0018 for ; Wed, 24 Jul 2024 12:10:31 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=f4mdULfc; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721822993; a=rsa-sha256; cv=none; b=RlgPhBJ5gef2kGkHZgCaw0AJOb4xBEy75RwtyFsvmz4V3uiL7s9Hd+SRnRL0n4firHJP0m yhrRLJcbz8c/jQo3LIVed95ahLgY8ZTOsCkCO9Lse84jjqM2rkQyUuvN3TEY5jLF2wpk51 p1QLZXC9lVea65mVP86prTjEdXiLxAA= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=f4mdULfc; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf28.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721822993; 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=pmjzYU2EWBaeA6HHqDKF1v64IPSOiylq5fYsLCB9w2k=; b=nv3Pnytxbq1C0UkbnEeE5TO5shHbURqF0BmQG90adJlsIH8r39P0EqzK5PT0jJbGQHHIU3 ht6dmKogrG+5CJNfs3OdvHlvpnW30Q6U0sOIM1+1XmpxEoHCITaVhKBmGB5q0MRQXXwuml XRmaivgfwwrYzXxs1+F2FIXqdJXNnSw= Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a7a9e25008aso209551266b.0 for ; Wed, 24 Jul 2024 05:10:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721823030; x=1722427830; 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=pmjzYU2EWBaeA6HHqDKF1v64IPSOiylq5fYsLCB9w2k=; b=f4mdULfcjJPZRwyy+yCoIX2yptLqiTclPlsMkg8GlIyjoXubCePnDTDEfO9wNm9jWV hNsQIqpLxyaDagJWQz08dmawJfVCg5NS2qoG+XAj8JJju8R4fh7eB40Gegopbx1mJPuF W6qD3VX51ulqTlhRBkVwc0evtFBHloSKPKFw94vwYc0gOyegGG0i2mQ+B9IQNwRoDjuP /0FjbSPoCFrnnJPj937PHffSCXUtarkf8etKduUYaj16NXRWtUP5wfi0l8z+mVF4MABI rZgS/ukLZnMd9BPABKWSsYJTTez1hvW7ue74sIDWGDNHWKGvOWDcbGwDVpACpUx8eHhw Dncg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721823030; x=1722427830; 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=pmjzYU2EWBaeA6HHqDKF1v64IPSOiylq5fYsLCB9w2k=; b=K3zuBAQYkvLX/by09ZFm3ru2CehSSzhw4bnmLl7XkKvveZAsCCLYD0GHydqCktTOa/ 2QiS0e4ZbIatP9w4ANR88DohvKsNetmJ/fTOZXYtVI0spdSGL0SfLwwPVJIqpJEsCpJ7 0S1KaTTz+YYCm05Hx1QLo2lpiD3iA5x+lo7N3tZcxzuGrwC8GAD+2ny2mK3fnFbf0miV z7ZnVgmTBpqPKTzwR95XPJ4gsp1EijoQyfsv4Fd2ONZyoOWU25Zz6HpNwIH0DBv1O9Wa VQ5bGbY3L9sqbgrJeMm2602Y3XBELv23tFNwa4y0fWQ5HOqD0O11qSwGN6R2Ngx1GtQX U89g== X-Forwarded-Encrypted: i=1; AJvYcCWHotUB8e8ZJ09Cn4DlQPz7tsEiOddf08Ur1UBvEEKLcefx5ToA/zDTU++52layDewxeCxtPkXFAj2VEPlm+iwgNYw= X-Gm-Message-State: AOJu0YwyWI5JMNdGQm2mmSIav9Z/jMAH91hx+Ckg/VU4lhnmE8ehDax2 M1EAs8+tlmrc7qM35wE0GzxsQ07czFOLCpTofHbVv4T6CKKfR2mti7gWVxjkDu8= X-Google-Smtp-Source: AGHT+IGzjeQyB8/dl1lPzvBVM7XthAbOmjwuVzyXWnY8xsK8r3+hn2IIGBuBNGkNEQhpk7RXNQ9QSg== X-Received: by 2002:a17:907:2d29:b0:a7a:a46e:dc39 with SMTP id a640c23a62f3a-a7ab094a9f3mr141553566b.0.1721823030481; Wed, 24 Jul 2024 05:10:30 -0700 (PDT) Received: from localhost (109-81-94-157.rct.o2.cz. [109.81.94.157]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a7a3c8bebe5sm632739266b.119.2024.07.24.05.10.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Jul 2024 05:10:30 -0700 (PDT) Date: Wed, 24 Jul 2024 14:10:29 +0200 From: Michal Hocko To: Barry Song <21cnbao@gmail.com> 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, penberg@kernel.org, rientjes@google.com, roman.gushchin@linux.dev, urezki@gmail.com, v-songbaohua@oppo.com, vbabka@suse.cz, virtualization@lists.linux.dev, hailong.liu@oppo.com, torvalds@linux-foundation.org, Kees Cook Subject: Re: [PATCH 3/5] mm: BUG_ON to avoid NULL deference while __GFP_NOFAIL fails Message-ID: References: <20240724085544.299090-1-21cnbao@gmail.com> <20240724085544.299090-4-21cnbao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240724085544.299090-4-21cnbao@gmail.com> X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 1D8DBC0018 X-Stat-Signature: i9w4g115ekc4mcne4ge3joqoqecg1x5t X-Rspam-User: X-HE-Tag: 1721823031-50204 X-HE-Meta: U2FsdGVkX1/ItF7bJmBzRRP1hwPPTpzi1lKPKOfajgMim2RqjodPX8XaBCqJ83vW3JiTNV7qBHCSx56SbqRKRWaHrADfcWy+0I9VWlWV65Az/s0JQ+H3pif/mRvOCYmc/nlwxR3w84u9N0GiEUqZ9RjY/FhcR7tZXtOk1H/Fq11hvhQhur2u6LK4Q4Gh41veqOLdL1PHV5zd1zENZ9foB4uTaHDGT55UufOtxTMNoAHJ1IQOLwwzSxdRWFM+vQCRiMLFRLlyeBkwNMyU9oAKPYWXOtJdNuyalqiJ25u6VUVwSiGqHQq0Oxoh6xXSrXb54c1GJKVte3zMKfcCvn5RpB1tgqy+RsDKChfdBOBfbJizEBOu0mp1uBQlrUl/sdNBxJr+KR0+6o5OULFDz7Eb+3Kc4ssLl/giGcppXpltoHcwI1ExCcmvqPChh8jlGthYqUueiFAdRkaybdNev08wYjBILM7UtdbF8lDRdbPmFqjk7EEDvWURCAQXQXGKo+2IYqtkERPfRb2PfD9XGFuyyBrJw88mDUBnQjm0SWvsXdJk9S0ZDc5GHKdUu+769Uy5XJnWi7TdVcm+egEE4Qn1qdW5zubyLkyz58RxzKYXYpSuxaY8ctIerpB17B0/xkG1b/Ch+WQ10SkWc7t0ws8HIkDpzVFDUuI104/01oXzay5NLBLcKDmbe0NlatAtvPEm6tkyG1vY5eRd0hvJ3tV/9Rvjn8YCtcXodRzkU/roJYch2D5hR1Bt2PJSG5HnGJiPgnELRmYXlxwlydQXFzgkkjyP00hEtH/5/rPbqCNXjQ7QoXlZNj/qLvIEPvlE5jqHrCrBPRVWuU6m5UeOrGLVIF+TBVmOdM3a70hBxS0cNmrhfmZJPKYfZU4/WRWi3kwg1mByNqiybU5pFzMOJBUWnRNnXfQNJAekwxV4d+KQ+7v8rXBXePKUwACjA0EYRtN2I4xJ1I9OWUQq4z3jRUb /ZAWuPmz cHmXALoUaW/JY/LQRcKlHWS++4Lk9bdF1Cn7qyRj0AGhEXtrEFvUiwNxWmHAWCf7uYIhujQ/fUE5sPDT2HtmVYCJfEFNHdw/fzB+7fnnXYfhIl/hDBOzopVaXAb59MfS80T/xmTX7bwJIeiglTDGK5+29V0TuNX/eVt7Z9LH4l9HWwjBw/H7RVToOGUTrfJ56ihNIJFaZ4sPsGbQpjpmQgYqsjnIPrxgE8KrUuCFmPnJxVdwGD5ptGJGePhODaUSjxBNfRUjG9ir2H4guFvAWuian6MZAns+Id+igEFcdjBbCk6eYQZCbLOu/psCTUg6Hhl+MQY+NQAp9go9qmOG3rJO2ka3dyASMio+VmH6JJkOGN6zlq4xUGupmSx6gxpfmse4ndUgfmZptsBrObxKO9/WerH5K4TJ3W5ic6OZC6P9qRptkRKqLPudwtatYWZtKXBLYV9/L2qggpwB/2brwqgZKTg== 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 24-07-24 20:55:42, 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 same > 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 failing). > > 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 to > blow up the kernel. Now the argument could be made that those failures > could cause subtle memory corruptions or even be exploitable which might > be a sufficient reason to stop them early. I think it is worth adding that size checks are not really actionable because they either cause unexpected failure or BUG_ON. It is not too much of a stretch to expect some of the user triggerable codepaths could hit this - e.g. when input is not checked properly. Silent failure is then a potential security risk. The page allocator, on the other hand, can chose to keep retrying even if that means that there is not reclaim going on and essentially cause a busy loop in the kernel space. That would eventually cause soft/hard lockup detector to fire (if an architecture offers a reliable one). So essentially there is choice between two bad solutions and you have chosen one that reliably bugs on rather than rely on something external to intervene. The reasoning for that should be mentioned in the changelog. [...] > 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(size, 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); I guess you want to switch the ordering. WARNING on top of BUG on seems rather pointless IMHO. > return NULL; > } > > -- > 2.34.1 -- Michal Hocko SUSE Labs