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 264A0C54EE9 for ; Sun, 25 Sep 2022 15:25:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 749928E001C; Sun, 25 Sep 2022 11:25:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6F7888E0007; Sun, 25 Sep 2022 11:25:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5BF138E001C; Sun, 25 Sep 2022 11:25:04 -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 50A468E0007 for ; Sun, 25 Sep 2022 11:25:04 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2B37740106 for ; Sun, 25 Sep 2022 15:25:04 +0000 (UTC) X-FDA: 79950980928.20.48B36E1 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf25.hostedemail.com (Postfix) with ESMTP id A1816A0006 for ; Sun, 25 Sep 2022 15:25:03 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id C629EB8098D; Sun, 25 Sep 2022 15:25:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2095C433D6; Sun, 25 Sep 2022 15:24:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664119500; bh=Fpv353I50hrgwd5/Rj0tOo2NjUe8ua59ZT8TMzWWfxs=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=U4HF8I6EvFuaKav5dBdbiPZx+L02Mon+78/CgcGbOW0upqxHMZNoLgshJetp2weMV DWQ8eYgvp679kV6qoD/ryKd/86IvDrhJ5fEnLcUtXhOaDx9PqrF/9CFbVAx2+wnSn8 I72hd8hd/m3U/vO9/jS+260HDc4PaTKKQyrsZsJq2EXMQpK1tuMUtjDAvWM5LbA3UL xLi9ioNc/K2AfmVbVCRM0RXpfDLJnevptsOEvOG7bgDbwLlRohzIPzINfxE9VFZjys D0jgw4eWq3uL4ArbeQk1kMTAUHKIvMUXLGGKeZb0wWuWk3yQp1IZQXCMra0snlxGR0 P7An+f/GY/XfQ== Message-ID: <449ccb5d-d2e5-950d-3458-5d0e56e83bf9@kernel.org> Date: Sun, 25 Sep 2022 23:24:55 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [PATCH] mm/slub: clean up create_unique_id() Content-Language: en-US To: Vlastimil Babka , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Hugh Dickins Cc: Chao Yu , Christophe JAILLET , Christoph Lameter , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220918092146.4060783-1-chao@kernel.org> <719a7ea0-c152-af03-8f6c-7dda702d1863@suse.cz> From: Chao Yu In-Reply-To: <719a7ea0-c152-af03-8f6c-7dda702d1863@suse.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664119503; a=rsa-sha256; cv=none; b=27vEDrANyyWIfZ1NTgc6Yq/Fi3XcBKEHKk6K5wpVxvh+jZ1ET22oWTa2OYG3QsyXqDJ34V tQ4gJtyDAs9FZ2y/11bxYt/vwYJz1wWhqHnDfs3jgeoS/sMUcdklw2ep63yZIs25Ny+cWD 0llAYJ4Z6U9Ra1KK1pKGfbenlY8pcgw= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=U4HF8I6E; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of chao@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=chao@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664119503; 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=vkZmWZIJOdVPDILEmxs+fHnlXKqUxBATk8u+qDRP+WA=; b=KRE1PeB/sGNA6Je44Qqj1pPX50VkDXGj1skoeO9ohbZ6XEdYT2wq8ifVZfAMRbkDcTw2AC lByEqy2E1pdWvydHB97nb03iumNLyyxu9vHpN4xq8KhREQJaTOYmXYabU6vHdbff6beXLC d2jmd8UMM9sVMbDiArnY0y4kOjBXRcM= Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=U4HF8I6E; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of chao@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=chao@kernel.org X-Rspam-User: X-Stat-Signature: b1b8oyc6xdsw1gwbjg5ymxsyd6p4dsjd X-Rspamd-Queue-Id: A1816A0006 X-Rspamd-Server: rspam06 X-HE-Tag: 1664119503-649526 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: On 2022/9/23 4:47, Vlastimil Babka wrote: > On 9/18/22 11:21, Chao Yu wrote: >> From: Chao Yu >> >> As Christophe JAILLET suggested: >> >> In create_unique_id(), >> >> "looks that ID_STR_LENGTH could even be reduced to 32 or 16. >> >> The 2nd BUG_ON at the end of the function could certainly be just >> removed as well or remplaced by a: >> if (p > name + ID_STR_LENGTH - 1) { >> kfree(name); >> return -E; >> } >> " >> >> According to above suggestion, let's do below cleanups: >> 1. reduce ID_STR_LENGTH to 32, as the buffer size should be enough; >> 2. remove BUG_ON() and return error if check condition is true. > > I'd leave a WARN_ON there as we really don't expect this to happen, so if it > does, we should be loud about it and not silently fail. Agreed. > >> Link: https://lore.kernel.org/linux-mm/2025305d-16db-abdf-6cd3-1fb93371c2b4@wanadoo.fr/ >> Suggested-by: Christophe JAILLET >> Fixes: 81819f0fc828 ("SLUB core") >> Signed-off-by: Chao Yu >> --- >> mm/slub.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index 5ba6db62a5ab..a045c1ca8772 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -5883,7 +5883,7 @@ static inline struct kset *cache_kset(struct kmem_cache *s) >> return slab_kset; >> } >> >> -#define ID_STR_LENGTH 64 >> +#define ID_STR_LENGTH 32 >> >> /* Create a unique string id for a slab cache: >> * >> @@ -5919,7 +5919,10 @@ static char *create_unique_id(struct kmem_cache *s) >> *p++ = '-'; >> p += sprintf(p, "%07u", s->size); > > Hm but sprintf() will happily overflow, so if we only detect that > afterwards, it's kinda too late to gracefully fail. > Should use snprintf() then? Correct. Updated in v2. Thanks, > >> - BUG_ON(p > name + ID_STR_LENGTH - 1); >> + if (p > name + ID_STR_LENGTH - 1) { >> + kfree(name); >> + return ERR_PTR(-EINVAL); >> + } >> return name; >> } >> >