Received: by 2002:a05:6a10:16a7:0:0:0:0 with SMTP id gp39csp4238075pxb; Tue, 10 Nov 2020 11:09:44 -0800 (PST) X-Google-Smtp-Source: ABdhPJz/kOcHHhFXlA4ZYxY/BhPhwavEM+znUCZWLeU3rS+PU0KHYmyPSM2RNkQMecmq1Do4Ry3E X-Received: by 2002:a17:906:ca83:: with SMTP id js3mr22506062ejb.42.1605035384421; Tue, 10 Nov 2020 11:09:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1605035384; cv=none; d=google.com; s=arc-20160816; b=v2OibNoEDoPzR7I67BjushvUm8uVBAJtm6kMl+F53ifdPKmBhbhrb/Pu2KQj8xmUT7 /UpGbKydFv+U5lzarHuDz88XaxU8OUECYlFkr0NLwZERj4esyanA5VBz/rmv9+NtST+B UZpgtHQL/12bNVGfyLtgBitszGF5XDlTzNHLF7EzuTcyvDggPRAfNjgw91Wg6w2Ql3tf czYXxf9R7y315wxQhyYckM5oU+wTIIj73/ABD4aaHDkrYg+SNEUkjYwwCgISCHXvxbk1 KRwiqNL7Dbj7SL/AWV8uEx0R5jWHpFBnQmAEUfqhX5UqvgG36izlxrQL5+YLSm9bw4Gt RHtQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject; bh=9OGyztXkfm/i/O40GHkOd2hSrWZW099OwP90TVJCErc=; b=niYcLBVKReqmE4Mcfmzl1eGtWkXEnc871ccVEnnIGB20KHBORY07itWMokv/1S530R bkfVGsQKiZuT+WSH6GldjNN3h5L66Ff1wh1yde9SHa7KvN5N7WeHPW4g23n893rn401q x9M0knxk+dTZqcQAo5WhPZVCPAjRurdXaeyJFdibbFegWKnuotSu2A3t7CS3TW/zZh0g hjD0HFW5SBCNMPe7Vz9rxzjGD8q6SxkfAEa31CZGv0WRv4FRxUSkGMmGhxRNlcwIoxTL 1idejXY/lQEnmono3HgpdGib5nor1dnK0oReE6LvzsdHGOw9NapKe6cdnycPrgaX/5OU c0Zw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=canonical.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id hr13si5611996ejc.429.2020.11.10.11.09.13; Tue, 10 Nov 2020 11:09:44 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731067AbgKJTHm (ORCPT + 99 others); Tue, 10 Nov 2020 14:07:42 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:51538 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726400AbgKJTHl (ORCPT ); Tue, 10 Nov 2020 14:07:41 -0500 Received: from 1.general.cking.uk.vpn ([10.172.193.212]) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1kcYzR-0002fO-Lu; Tue, 10 Nov 2020 19:07:37 +0000 Subject: Re: [PATCH][next] cpumask: allocate enough space for string and trailing '\0' char To: paulmck@kernel.org Cc: Paul Gortmaker , Qian Cai , Stephen Rothwell , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, Linux Next Mailing List References: <20201109130447.2080491-1-colin.king@canonical.com> <737d5be9eb5af55b1a61bd8bfb49b1829a3ff916.camel@redhat.com> <20201110152437.GS3249@paulmck-ThinkPad-P72> <6050d075-52cc-d1b8-51c4-4d0dac62a42e@canonical.com> <20201110183826.GV3249@paulmck-ThinkPad-P72> From: Colin Ian King Message-ID: <431c72da-1f85-ed4f-1023-08c844d572ac@canonical.com> Date: Tue, 10 Nov 2020 19:07:37 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.1 MIME-Version: 1.0 In-Reply-To: <20201110183826.GV3249@paulmck-ThinkPad-P72> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/11/2020 18:38, Paul E. McKenney wrote: > On Tue, Nov 10, 2020 at 03:34:05PM +0000, Colin Ian King wrote: >> On 10/11/2020 15:24, Paul E. McKenney wrote: >>> On Mon, Nov 09, 2020 at 11:57:15PM -0500, Paul Gortmaker wrote: >>>> >>>> >>>> On 2020-11-09 8:07 p.m., Qian Cai wrote: >>>>> On Mon, 2020-11-09 at 13:04 +0000, Colin King wrote: >>>>>> From: Colin Ian King >>>>>> >>>>>> Currently the allocation of cpulist is based on the length of buf but does >>>>>> not include the addition end of string '\0' terminator. Static analysis is >>>>>> reporting this as a potential out-of-bounds access on cpulist. Fix this by >>>>>> allocating enough space for the additional '\0' terminator. >>>>>> >>>>>> Addresses-Coverity: ("Out-of-bounds access") >>>>>> Fixes: 65987e67f7ff ("cpumask: add "last" alias for cpu list specifications") >>>>> >>>>> Yeah, this bad commit also introduced KASAN errors everywhere and then will >>>>> disable lockdep that makes our linux-next CI miserable. Confirmed that this >>>>> patch will fix it. >>>> >>>> I appreciate the reports reminding me why I hate touching string handling. >>>> >>>> But let us not lose sight of why linux-next exists. We want to >>>> encourage code to appear there as a sounding board before it goes >>>> mainline, so we can fix things and not pollute mainline git history >>>> with those trivialities. >>>> >>>> If you've decided to internalize linux-next as part of your CI, then >>>> great, but do note that does not elevate linux-next to some pristine >>>> status for the world at large. That only means you have to watch more >>>> closely what is going on. >>>> >>>> If you want to declare linux-next unbreakable -- well that would scare >>>> away others to get the multi-arch or multi-config coverage that they may >>>> not be able to do themselves. We are not going to do that. >>>> >>>> I have (hopefully) fixed the "bad commit" in v2 -- as part of the >>>> implicit linux-next rule "you broke it, you better fix it ASAP". >>>> >>>> But "bad" and "miserable" can be things that might scare people off of >>>> making use of linux-next for what it is meant to be for. And I am not >>>> OK with that. >>> >>> They would need to use much stronger language to scare me off. That said, >>> what on earth is the point of running tests if they do not from time to >>> time find bugs? ;-) >> >> For me, part of the QA process is statically analyzing linux-next to >> catch bugs before they land in linux. I think other testing is equally >> worth while as catching bugs early saves time and money. > > All kidding aside, the fact that this appeared in -next was due to a > mistake on my part, namely failing to push the changes before starting > the test. Please accept my apologies, and I will continue to do my > best to avoid this sort of thing. > > Thanx, Paul No problem. I'm glad we have tools to catch issues like this. Colin > >> Colin >> >>> >>>> Thanks, >>>> Paul. >>>> -- >>>> >>>>> >>>>>> Signed-off-by: Colin Ian King >>>>>> --- >>>>>> lib/cpumask.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/lib/cpumask.c b/lib/cpumask.c >>>>>> index 34ecb3005941..cb8a3ef0e73e 100644 >>>>>> --- a/lib/cpumask.c >>>>>> +++ b/lib/cpumask.c >>>>>> @@ -185,7 +185,7 @@ int __ref cpulist_parse(const char *buf, struct cpumask >>>>>> *dstp) >>>>>> { >>>>>> int r; >>>>>> char *cpulist, last_cpu[5]; /* NR_CPUS <= 9999 */ >>>>>> - size_t len = strlen(buf); >>>>>> + size_t len = strlen(buf) + 1; >>>>>> bool early = !slab_is_available(); >>>>>> if (!strcmp(buf, "all")) { >>>>> >>