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 73787C3DA7F for ; Wed, 31 Jul 2024 07:11:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0CF1E6B0088; Wed, 31 Jul 2024 03:11:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 07EFC6B008A; Wed, 31 Jul 2024 03:11:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E886D6B0092; Wed, 31 Jul 2024 03:11:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CCD956B0088 for ; Wed, 31 Jul 2024 03:11:35 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 722F7A0173 for ; Wed, 31 Jul 2024 07:11:35 +0000 (UTC) X-FDA: 82399177350.07.2C99F34 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by imf18.hostedemail.com (Postfix) with ESMTP id 80AC81C000D for ; Wed, 31 Jul 2024 07:11:33 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=SL9M5Ivl; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf18.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.44 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722409852; a=rsa-sha256; cv=none; b=FnfR5FFLlU5BG/JmGRkQcTdQcWvNaIWJSSGiQJSt0a+Th1piNuDZNgxA8eJvjKxbunZS3i ujup/EYUaLNHdSH8lkhCanzbqsx7kuCZw5aaZRwqJW+uziP1voAm9aRqiBjsqVBfVOckoM 1vqPVMVbfyuWJaAT136tByW0JDHN6s4= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=SL9M5Ivl; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf18.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.44 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=1722409852; 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=C7bCfmlE877BhmRYlIJ409tyg+mr+Mk0Kk/ub9KupKM=; b=W2Rt9j9TIQhxzI2n0G04Hy88rXpS1ZzfBCaI1yppUK59cife03Ez+tdxUbSTBCYk3g9Zp8 yOMfHtq3qvMaV+t4kWyN4jd6pg8i+Q7vmOPNerBSEIB2qs6zf0d3d0xC21h1wr/6uqee0V lCSv+p/VJ/W5ogQvNLLcVIlMPc0Appw= Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-4280b3a7efaso34955435e9.0 for ; Wed, 31 Jul 2024 00:11:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1722409892; x=1723014692; 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=C7bCfmlE877BhmRYlIJ409tyg+mr+Mk0Kk/ub9KupKM=; b=SL9M5IvlF9wthBTvDg0T7vzvY468FWg9aPgXPvsPPVOrxD7uW8BltMYtMcL1bm4ZmU 79Djui1a5xwyXjaXF/MMmfvTXNm1+NOTzI+oadB1UXaielfpVNQFt4i+RNzEN5ereLWT SFgnN8MS18BU2sM9DtPVYHNCqU9MJ2UwVmpn00FKxPnP3eF6et7RTsE6yz4bgQYPyKD+ Jcfj+oCsS0oC9mb9TEOCsmIGIjIrtO7gnJQ2ZWXO+AFWQiLvWqkYiXlXCfuP1KzCPpyc fGriNgsji8+p8Tmcj3BUipXKBpRb2JxmGu97Z5cl9/IU/SdIrPYw1AXwd7Kdwed+v5rL ZefA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722409892; x=1723014692; 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=C7bCfmlE877BhmRYlIJ409tyg+mr+Mk0Kk/ub9KupKM=; b=hwD4GXEB4Fegoct9TsEUBYMlLAZJ8CcGjHmImwfb/5F1bjmo5taEfi8tj5byv64p7Y Fz+JUSdudj1bAh9Q18hJB4nOCn3XJ1AaZ78BG5QH8wd58VzcpdrK+9gEhSkoxR7Wrz10 F7STv+C/G5Nalz3xuHFgt0w2teQV+bClCYfTO+Cb9b3T6tmkWyyMklUl3SZDV8F3axBd NeE3hqCCnnG7tT613vnJPJXNnkBWmXhHGkZsMB/Q+3ODS6QS92fG8FpyoJI2/R9Hx+aZ tzSKSo5bz/xZsQmiZPiGer/6AZ1uOlV5YyG42JVhv8JAEtYsSKAM5EZ2BydC6X4yF5IB LoDg== X-Forwarded-Encrypted: i=1; AJvYcCWJHW4kvPBkoRVB2d/EnW73UVpneWuQ0N/4+xHMaejhSXUjX0dPPRvxJopw1733lTA2xoH9Z4UhZDxVz98xTdY8YjQ= X-Gm-Message-State: AOJu0Yxzp3vKq8JSDzjvL/+hi6wHFadvjf9PR4run0rkq1JGf1kVNaoR 86jNCMSXuuMGCaYyG37kV0d/xn3R2l7+kbj2bXLDwkqIQ/dGh2X1BEHax3OAu90= X-Google-Smtp-Source: AGHT+IFPHmzgv0qlkyM4TUCyPsg31JI0gzU3+iiXja8QaEmxeboDo2tgkRQ+Sk8hCi2NE9a/jiP2yg== X-Received: by 2002:adf:e90b:0:b0:360:7812:6abc with SMTP id ffacd0b85a97d-36b5d0cc62cmr8730220f8f.60.1722409891631; Wed, 31 Jul 2024 00:11:31 -0700 (PDT) Received: from localhost (109-81-83-231.rct.o2.cz. [109.81.83.231]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36b36857fd1sm16293815f8f.69.2024.07.31.00.11.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jul 2024 00:11:31 -0700 (PDT) Date: Wed, 31 Jul 2024 09:11:30 +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, hailong.liu@oppo.com, hch@infradead.org, iamjoonsoo.kim@lge.com, lstoakes@gmail.com, penberg@kernel.org, rientjes@google.com, roman.gushchin@linux.dev, torvalds@linux-foundation.org, urezki@gmail.com, v-songbaohua@oppo.com, vbabka@suse.cz, virtualization@lists.linux.dev, Kees Cook Subject: Re: [PATCH v2 3/4] mm: BUG_ON to avoid NULL deference while __GFP_NOFAIL fails Message-ID: References: <20240731000155.109583-1-21cnbao@gmail.com> <20240731000155.109583-4-21cnbao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240731000155.109583-4-21cnbao@gmail.com> X-Rspamd-Queue-Id: 80AC81C000D X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: gs4drnphg1qt5j1shd6hfdwjgzd59wjm X-HE-Tag: 1722409893-670081 X-HE-Meta: U2FsdGVkX1/zlt/EmxIShHeCqSCNGugkhqzrKSa0OWEL6oqidqS3pSPU/CfMvGp6VLt8KcIS71ZxL0RnrfcGT88gLLkPxnz8A/BIkH8ZVciIdRQUUKez3VV85qcxWPrTfEzg849kaZlrxOq6gpXYTd81MGrsFUHNkn+wbL+5gJuFgwWmOr0qSsgqZ0cBAnZjuC+IAn08wa3XIINtAauHtL2N5ZSZVPTsIAPzJGv/6drib1hZQ0Ticr9ZpaJjIIJwUdxFJnGF/eGr3VGyGqlCSui+k4lDX7HwUx0cZpvxe0OKZBz6beweo02KLWvRRjtzM6+oFkCptnSMtRVZmz1w5sao2UPNtX72Zt9eA5My01SHX1JvzozERQCXW/kkqSl/OdXI6uXlMzygBQwFx60qi2FkvV3R4EQpq4jvKEsSjp8SO0KZ97issiFnmZz/fDpX1dqCYUsm7iTrWO8yuN5wse+Phn2Ad0qhWgcmjooQb8KLJVxVU+B1O2v/gXJkVNP9DZMzpZ/r56YmxPGWrZP56nfbTIkvk06wp5U59lPMNooYhKRsB0jyq0T3OahB4cMV4Vxk7EIxrV6wLm3FsHZIEtIsiW3/sdBJPSHpLVkqEwBJjcIvZ5ivldEQmFDED2g7BJGC3TOB3+uKIqIUxiV1oMp/Hr2HkpxGydVZazPkL43YZHUW7WBv6lR5GwCvxEBPUCT691vLc2RLTNg3sPHGzz4AudFVqlixj7uSnGNLkel87EuqQGE/9D5daqE6QP4KBwEGHKqLGCBII9hQkTVHzN9CIASUcYCJAdCS5jE89tDyDxQpF8EnPPzBWQ8G1Ur7Mp0MZhJpXhGgsJlDDZTzJFv3xmcclgBHWFUbo+I6E8PF9ZOAwEbdYaV9sFOlSv38T4GzhQiHnkNXq4VbIsfKfQjINli+s2y0XH4ApIUBxpdVKx5GxmUwyHVv41Ia8uCvtMEVdPBmNKZN3q7Nnej 8SaCa2tC C0TC71grJZH1gSNWm+acaDRG5M3SqO0VS4SpCA6a+9/Rx2gGSwAGyOvrGk8bcJQQA9vfvSCZlSCZoaPGPuaAzNBRxe1SQ8t/M9yr8mbGeRtu5SC6WXo6HJilVgI6mL2oKfypKO+37ZdT8+kseZJDBglTdBfDccjWzKxLmI++viPv9JAQr0dHHM1ZAn9RK3b9DPNg9dcj2SbQGQfBRhJX7jjZsZkghpQtvuqAVsnU1ScB5WS1FTkV0dIE37L969HmHBJsTMVhJoqFxVYxFfcMvZbDuj/TUYg8NbTB6lNTcMOkaMm+M8ce4ei5tdswMZ20hT4dJf+nhQNNVeeH0buuAp5ac2p6YgRBcSpvWHW8f/0HxjEYN38HWxrPxzXLE9oyxQapT0btjwtjMFAcKOfutc+fMRiCQcfPIrnEY6eeRbgTbBFMe/vTK4NgSxb6NHJVxPyycdbe57CHZBcvXszfdx13Z1CJSEN6CUGNHudDUjS1tNRMUO7G1EiMti1Y1hAFgNTSehZqVJ+R+4HuF/KoiHvd0joCEG5PUuV4qELlCGFxQGKNcfs1+vrKSGu4hmykPPtONSFR/6kdHKHZqAUieygYBTlC02VfK1ViwF6ywtbjOcZR5x4BSHNdZPeSLBkh7ygslkSMGhdOXk38fqEiKOxq85rAkDFBb9eyRPN2jw4Kz3XvS0GNEoOOmuEFRMIsw6Mnqdkuo2qVuYARYfKRdGHxj92mXp9WjnILVnbhRwyUrf/38pA2SskVWs9xIButjFnWYK8xXQUqaNm7jMUiIzpdSkMr/kPqmLG2ExlXi/0jeMCiPljJLPecprQugphYpbVbqwtfN8SiNWk5tUNE0MaGQS/KzXWF46DibB7x/FKlpzrU= 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 31-07-24 12:01:54, 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. > > 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 Thanks for separating overflow/out of bounds checks. Acked-by: Michal Hocko > --- > include/linux/slab.h | 4 +++- > mm/page_alloc.c | 4 +++- > mm/util.c | 1 + > 3 files changed, 7 insertions(+), 2 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 c700d2598a26..cc179c3e68df 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4708,8 +4708,10 @@ struct page *__alloc_pages_noprof(gfp_t gfp, unsigned int order, > * There are several places where we assume that the order value is sane > * so bail out early if the request is out of bound. > */ > - if (WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp)) > + if (WARN_ON_ONCE_GFP(order > MAX_PAGE_ORDER, gfp)) { > + BUG_ON(gfp & __GFP_NOFAIL); > return NULL; > + } > > gfp &= gfp_allowed_mask; > /* > diff --git a/mm/util.c b/mm/util.c > index 0ff5898cc6de..bad3258523b6 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -667,6 +667,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)) { > + BUG_ON(flags & __GFP_NOFAIL); > WARN_ON_ONCE(!(flags & __GFP_NOWARN)); > return NULL; > } > -- > 2.34.1 -- Michal Hocko SUSE Labs