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 8F45CC433F5 for ; Thu, 2 Dec 2021 04:45:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BD1B26B0072; Wed, 1 Dec 2021 23:45:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B80E86B0073; Wed, 1 Dec 2021 23:45:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A6F9E6B0074; Wed, 1 Dec 2021 23:45:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0167.hostedemail.com [216.40.44.167]) by kanga.kvack.org (Postfix) with ESMTP id 984556B0072 for ; Wed, 1 Dec 2021 23:45:32 -0500 (EST) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4F8BE8249980 for ; Thu, 2 Dec 2021 04:45:22 +0000 (UTC) X-FDA: 78871615284.02.0B4A573 Received: from out4436.biz.mail.alibaba.com (out4436.biz.mail.alibaba.com [47.88.44.36]) by imf18.hostedemail.com (Postfix) with ESMTP id 8A2D74002085 for ; Thu, 2 Dec 2021 04:45:20 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R171e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e01424;MF=cuibixuan@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0Uz8X8gR_1638420291; Received: from 30.43.84.45(mailfrom:cuibixuan@linux.alibaba.com fp:SMTPD_---0Uz8X8gR_1638420291) by smtp.aliyun-inc.com(127.0.0.1); Thu, 02 Dec 2021 12:44:52 +0800 Content-Type: multipart/alternative; boundary="------------RurqEC8VdAad7ksTKOza5cC7" Message-ID: Date: Thu, 2 Dec 2021 12:44:51 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 Subject: Re: [PATCH -next] mm: delete oversized WARN_ON() in kvmalloc() calls To: Kees Cook Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, leon@kernel.org, akpm@linux-foundation.org, w@1wt.eu References: <1638410784-48646-1-git-send-email-cuibixuan@linux.alibaba.com> <202112011944.28EF2FC44@keescook> From: Bixuan Cui In-Reply-To: <202112011944.28EF2FC44@keescook> X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 8A2D74002085 Authentication-Results: imf18.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf18.hostedemail.com: domain of cuibixuan@linux.alibaba.com designates 47.88.44.36 as permitted sender) smtp.mailfrom=cuibixuan@linux.alibaba.com X-Stat-Signature: tmookc78w5ix4gyyu8jbonyocj49zap7 X-HE-Tag: 1638420320-399957 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: This is a multi-part message in MIME format. --------------RurqEC8VdAad7ksTKOza5cC7 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable =E5=9C=A8 2021/12/2 =E4=B8=8A=E5=8D=8811:46, Kees Cook =E5=86=99=E9=81=93= : > On Thu, Dec 02, 2021 at 10:06:24AM +0800, Bixuan Cui wrote: >> Delete the WARN_ON() and return NULL directly for oversized parameter >> in kvmalloc() calls. >> Also add unlikely(). >> >> Fixes: 7661809d493b ("mm: don't allow oversized kvmalloc() calls") >> Signed-off-by: Bixuan Cui >> --- >> There are a lot of oversize warnings and patches about kvmalloc() call= s >> recently. Maybe these warnings are not very necessary. > It seems these warnings are working, yes? i.e. we're finding the places > where giant values are coming in? Yes,=C2=A0 It's working. > >> https://lore.kernel.org/all/YadOjJXMTjP85MQx@unreal >> >> The example of size check in __do_kmalloc_node(): >> __do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long ca= ller) >> { >> struct kmem_cache *cachep; >> void *ret; >> >> if (unlikely(size > KMALLOC_MAX_CACHE_SIZE)) >> return NULL; >> cachep =3D kmalloc_slab(size, flags); >> >> mm/util.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/util.c b/mm/util.c >> index 7e433690..d26f19c 100644 >> --- a/mm/util.c >> +++ b/mm/util.c >> @@ -587,7 +587,7 @@ void *kvmalloc_node(size_t size, gfp_t flags, int = node) >> return ret; >> =20 >> /* Don't even allow crazy sizes */ >> - if (WARN_ON_ONCE(size > INT_MAX)) >> + if (unlikely(size > INT_MAX)) >> return NULL; > If we're rejecting the value, then it's still a pathological size, so > shouldn't the check be happening in the caller? I think the WARN is > doing exactly what it was supposed to do: find the places where bad > sizes can reach vmalloc. In this way, we must check whether the size from the user exceeds INT_MAX before calling kvmalloc() calls.=C2=A0 Generally speaking, the oversize c= heck=20 is rarely done before. Thanks, Bixuan Cui > > -Kees > >> =20 >> return __vmalloc_node(size, 1, flags, node, >> --=20 >> 1.8.3.1 >> > -- Kees Cook --------------RurqEC8VdAad7ksTKOza5cC7 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


=E5=9C=A8 2021/12/2 =E4=B8=8A=E5=8D=88= 11:46, Kees Cook =E5=86=99=E9=81=93:
On Thu, Dec 02, 2021 at 10=
:06:24AM +0800, Bixuan Cui wrote:

Delete the WARN_ON() and=
 return NULL directly for oversized parameter
in kvmalloc() calls.
Also add unlikely().

Fixes: 7661809d493b ("mm: don't allow oversized kvmalloc() calls")
Signed-off-by: Bixuan Cui <cuibixuan@li=
nux.alibaba.com>
---
There are a lot of oversize warnings and patches about kvmalloc() calls
recently. Maybe these warnings are not very necessary.

It seems these warnings ar=
e working, yes? i.e. we're finding the places
where giant values are coming in?
Yes,=C2=A0 It's working.


https://lore.kernel.org/all/YadOjJXMTjP85MQx@unre=
al

The example of size check in __do_kmalloc_node():
__do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long calle=
r)
{
        struct kmem_cache *cachep;
        void *ret;

        if (unlikely(size > KMALLOC_MAX_CACHE_SIZE))
                return NULL;
        cachep =3D kmalloc_slab(size, flags);

 mm/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/util.c b/mm/util.c
index 7e433690..d26f19c 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -587,7 +587,7 @@ void *kvmalloc_node(size_t size, gfp_t flags, int nod=
e)
 		return ret;
=20
 	/* Don't even allow crazy sizes */
-	if (WARN_ON_ONCE(size > INT_MAX))
+	if (unlikely(size > INT_MAX))
 		return NULL;

If we're rejecting the val=
ue, then it's still a pathological size, so
shouldn't the check be happening in the caller? I think the WARN is
doing exactly what it was supposed to do: find the places where bad
sizes can reach vmalloc.
In this way, we must check whether the size from the user exceeds INT_MAX
before calling kvmalloc() calls.=C2=A0 Generally speaking, the oversize check is rarely
done before.

Thanks,
Bixuan Cui



-Kees


=20
 	return __vmalloc_node(size, 1, flags, node,
--=20
1.8.3.1


--=20
Kees Cook
--------------RurqEC8VdAad7ksTKOza5cC7--