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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CDC21C433FE for ; Thu, 3 Nov 2022 16:31:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232174AbiKCQbg (ORCPT ); Thu, 3 Nov 2022 12:31:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232138AbiKCQbD (ORCPT ); Thu, 3 Nov 2022 12:31:03 -0400 Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D2081B9D6; Thu, 3 Nov 2022 09:30:57 -0700 (PDT) Received: by mail-oo1-xc36.google.com with SMTP id s125-20020a4a5183000000b0047fbaf2fcbcso327353ooa.11; Thu, 03 Nov 2022 09:30:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:from:to:cc:subject:date:message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=Jxprr5Yb6GFP2iMDdNYwrv+edL5lsVZ+sskHoEYasypEvTLy4Q6vhTS7qqFC2ZOJ9F 7UlwmUBWdL8ATQXLGSU2oLt77vVHPXn0OrJO9RZ4ozCxeZSFNH6CKpqcZtCvnwol4zeN srTpQmrtC9fSlFcJoeE3VBMkcRHxF4UC77PI0QPdBZOTiLk4O1EgX10WGEBp+gELT/Of f0hLEd5d1y3j2mzfUulOKKw9wLwe0rGXdFxYErQK/3o71DkASuqcP1UgrC5XDCQfhx1n GSTmaFTFIU7py0xRt57Y61rOd+D7tYEKoK35ZenJvXOnTlTZe3nUPKJMZ1OW7PKQwbP+ CarQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=xeSml70lzdk6Im+gznsgmRpQEqFrjzvZ8ZDIs3UDsAQ8Zu6AfPwNOjDuw905JyBM0I fxEyt7Duk6uEGz8xuDpSCY0LXwbYdRJqbSJLo+D4JclTnOjlEJyJj98mmq2u6HcKu1NE NVkqCYcvAOgi73ws5DW+LZuT+ycAk51lXMxwDYyPDeQeCmK14H8rEdeGKYS7lr7ymQQz DbkrPaKiS9C/Lw+O0kiyI5Bl0IhVmZUHf7msJgc8hC80WT2ERNnXzVYfUVLAa+TAM+ry QtEfhJIZn6F8XASs9rrua86uyhBgZU1Wejq+lCL2zPTPFMCxl5eQ2ke3RXiYSNGYvc8P lC1A== X-Gm-Message-State: ACrzQf2k14Wv/JL70i0Vpgg2bu5A5VQvEUMD0L9KVa4TFx75EuYfrL3y ImreOytLYgILP/0sdEyLoz8= X-Google-Smtp-Source: AMsMyM4XFkvxtx6hYtImOQooUBUuMwcsCe7CNpBD20J79Sc2FfnE52Rm7s0RT965HybTqZXYxmJ8ng== X-Received: by 2002:a4a:a609:0:b0:499:74ab:f10e with SMTP id e9-20020a4aa609000000b0049974abf10emr12101601oom.13.1667493056510; Thu, 03 Nov 2022 09:30:56 -0700 (PDT) Received: from localhost ([12.97.180.36]) by smtp.gmail.com with ESMTPSA id t15-20020a4a858f000000b00480e1109695sm370965ooh.10.2022.11.03.09.30.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 09:30:56 -0700 (PDT) From: yury.norov@gmail.com X-Google-Original-From: Yury Norov , To: Andrew Jones ; Date: Thu, 3 Nov 2022 09:30:54 -0700 Cc: Borislav Petkov , x86@kernel.org, linux-riscv , Linux Kernel Mailing List , Thomas Gleixner , Ingo Molnar , Dave Hansen , Palmer Dabbelt , Paul Walmsley , Albert Ou , Jonas Bonn , Stefan Kristiansson , Stafford Horne , openrisc@lists.librecores.org, Michael Ellerman , "open list:LINUX FOR POWERPC PA SEMI PWRFICIENT" , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , linux-s390@vger.kernel.org Subject: Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning Message-ID: References: <20221031080604.6xei6c4e3ckhsvmy@kamzik> <20221031100327.r7tswmpszvs5ot5n@kamzik> <20221103125945.lrr5oxxmylwpam53@kamzik> <20221103153404.uh77nrdkowrxj6cr@kamzik> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221103153404.uh77nrdkowrxj6cr@kamzik> To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 03, 2022 at 04:34:04PM +0100, Andrew Jones wrote: > On Thu, Nov 03, 2022 at 04:02:12PM +0100, Borislav Petkov wrote: > > On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > > > The patch I'm proposing ensures cpumask_next()'s range, which is actually > > > [-1, nr_cpus_ids - 1), > > > > Lemme make sure I understand it correctly: on the upper boundary, if you > > supply for n the value nr_cpu_ids - 2, then it will return potentially > > the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). > > > > If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no > > further bits set. > > > > Yes, no? > > Yes > > > > > > I'll send a v4 with another stab at the commit message. > > > > Yes, and it is still an unreadable mess: "A kernel compiled with commit > > ... but not its revert... " Nope. > > > > First make sure cpumask_next()'s valid accepted range has been settled > > upon, has been explicitly documented in a comment above it and then I'll > > take a patch that fixes whatever is there to fix. > > That's fair, but I'll leave that to Yury. I'll take care of it. > > Callers should not have to filter values before passing them in - the > > function either returns an error or returns the next bit in the mask. > > That's reasonable, but cpumask folk probably need to discuss it because > not all cpumask functions have a return value where an error may be > placed. Callers should pass sane arguments into internal functions if they expect sane output. The API not exported to userspace shouldn't sanity-check all inputs arguments. For example, cpumask_next() doesn't check srcp for NULL. However, cpumask API is exposed to drivers, and that's why optional cpumask_check() exists. (Probably. It has been done long before I took over this.) Current *generic* implementation guarantees that out-of-region offset would prevent cpumask_next() from dereferencing srcp, and makes it returning nr_cpu_ids. This behavior is expected by many callers. However, there is a couple of non-generic cpumask implementations, and one of them is written in assembler. So, the portable code shouldn't expect from cpumasks more than documentation said: for a _valid_ offset cpumask_next() returns next set bit or >= nr_cpu_ids. cpumask_check() has been broken for years. Attempting to fix it faced so much resistance, that I had to revert the patch. Now there's ongoing discussion whether we need this check at all. My opinion is that if all implementations of cpumask (more precisely, underlying bitmap API) are safe against out-of-range offset, we can simply remove cpumask_check(). Those users, like cpuinfo, who waste time on useless last iteration will bear it themselves. Thanks, Yury 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 72D03C4332F for ; Thu, 3 Nov 2022 16:31:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:Date:From:Reply-To:To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GxQv0y95J+WORfh4+a+2x3j41IlgRXIm5UvJSkMErNU=; b=Vo3qt1B6IhjXAA 1etpO8bV6zRp5PI8hzW7JFq53/8g0GZPRegCTV+/Ygu8QtrTNDO1A2GSNm9ihmmhTGCkx9zne2bxK zVM6sbO+/PfwjrHSBKy2jgDePGKhNcHSw3GEPfphTNDy07tk/JmpsNur3ICuZ+bKxjgWTW6ROFhlQ P9InCACBxtk5rc67JmbhnntJAT6Zn4hgSw+JMPv1BpZJoVugNAQTmHrdzBfRkQetM53v4mCM4mCur t76RqLUSEFHYwCo+/sjSdagAqV15XsCNpyOvB3Z7lR4WcBA6awYFWXS/DdZO7qezp+L9oTQCBqnu9 PBRV/4j8E8MSgRxvB3LQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqd7S-000hsy-4U; Thu, 03 Nov 2022 16:31:06 +0000 Received: from mail-oo1-xc35.google.com ([2607:f8b0:4864:20::c35]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oqd7N-000hpA-H8 for linux-riscv@lists.infradead.org; Thu, 03 Nov 2022 16:31:03 +0000 Received: by mail-oo1-xc35.google.com with SMTP id e11-20020a4ab14b000000b0049be568062bso333511ooo.4 for ; Thu, 03 Nov 2022 09:30:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:from:to:cc:subject:date:message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=Jxprr5Yb6GFP2iMDdNYwrv+edL5lsVZ+sskHoEYasypEvTLy4Q6vhTS7qqFC2ZOJ9F 7UlwmUBWdL8ATQXLGSU2oLt77vVHPXn0OrJO9RZ4ozCxeZSFNH6CKpqcZtCvnwol4zeN srTpQmrtC9fSlFcJoeE3VBMkcRHxF4UC77PI0QPdBZOTiLk4O1EgX10WGEBp+gELT/Of f0hLEd5d1y3j2mzfUulOKKw9wLwe0rGXdFxYErQK/3o71DkASuqcP1UgrC5XDCQfhx1n GSTmaFTFIU7py0xRt57Y61rOd+D7tYEKoK35ZenJvXOnTlTZe3nUPKJMZ1OW7PKQwbP+ CarQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=61Si2oa2XoDjRj2KnLCYEcyT1AB/mReS9w3nzi7iwnxt6YkjTaSS+M2MABwZbp1uVm xDpFvNhrp/F7tLXFecwkqj5XUtMmUHlQXRGZeGJ6yoJiYMTKKr02AfMcfFC7mP53BTGG e4jR3Wd9Uw2kandwqIZ4fS5RzJ9dccP8eY5nfWF1YTivAlSRF0RZ23Uvb0Q9KglpSgjW qPmC14iXJmrUgh1mSQLKJhtj4ZjjMq6snK3LdXmEliJIUmRIgGmB4OYT1hk3nXuq5yRa hPwhqwseu8C40kh54IX0eQTUjfg21kWi/ea6p3ljDbn97SC5jnCJwbu1BZwnwFQpK+T9 8aSQ== X-Gm-Message-State: ACrzQf1aj+8qoNgpJs75sACwxqgZ/cnSA2+O1itBT5NhZT4eFXBkzZK3 XTtdCvhIR/BWGY5EQ9ALIqM= X-Google-Smtp-Source: AMsMyM4XFkvxtx6hYtImOQooUBUuMwcsCe7CNpBD20J79Sc2FfnE52Rm7s0RT965HybTqZXYxmJ8ng== X-Received: by 2002:a4a:a609:0:b0:499:74ab:f10e with SMTP id e9-20020a4aa609000000b0049974abf10emr12101601oom.13.1667493056510; Thu, 03 Nov 2022 09:30:56 -0700 (PDT) Received: from localhost ([12.97.180.36]) by smtp.gmail.com with ESMTPSA id t15-20020a4a858f000000b00480e1109695sm370965ooh.10.2022.11.03.09.30.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 09:30:56 -0700 (PDT) From: yury.norov@gmail.com X-Google-Original-From: Yury Norov , To: Andrew Jones ; Date: Thu, 3 Nov 2022 09:30:54 -0700 Cc: Borislav Petkov , x86@kernel.org, linux-riscv , Linux Kernel Mailing List , Thomas Gleixner , Ingo Molnar , Dave Hansen , Palmer Dabbelt , Paul Walmsley , Albert Ou , Jonas Bonn , Stefan Kristiansson , Stafford Horne , openrisc@lists.librecores.org, Michael Ellerman , "open list:LINUX FOR POWERPC PA SEMI PWRFICIENT" , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , linux-s390@vger.kernel.org Subject: Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning Message-ID: References: <20221031080604.6xei6c4e3ckhsvmy@kamzik> <20221031100327.r7tswmpszvs5ot5n@kamzik> <20221103125945.lrr5oxxmylwpam53@kamzik> <20221103153404.uh77nrdkowrxj6cr@kamzik> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221103153404.uh77nrdkowrxj6cr@kamzik> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221103_093101_617064_DE4C0F26 X-CRM114-Status: GOOD ( 27.25 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Thu, Nov 03, 2022 at 04:34:04PM +0100, Andrew Jones wrote: > On Thu, Nov 03, 2022 at 04:02:12PM +0100, Borislav Petkov wrote: > > On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > > > The patch I'm proposing ensures cpumask_next()'s range, which is actually > > > [-1, nr_cpus_ids - 1), > > > > Lemme make sure I understand it correctly: on the upper boundary, if you > > supply for n the value nr_cpu_ids - 2, then it will return potentially > > the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). > > > > If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no > > further bits set. > > > > Yes, no? > > Yes > > > > > > I'll send a v4 with another stab at the commit message. > > > > Yes, and it is still an unreadable mess: "A kernel compiled with commit > > ... but not its revert... " Nope. > > > > First make sure cpumask_next()'s valid accepted range has been settled > > upon, has been explicitly documented in a comment above it and then I'll > > take a patch that fixes whatever is there to fix. > > That's fair, but I'll leave that to Yury. I'll take care of it. > > Callers should not have to filter values before passing them in - the > > function either returns an error or returns the next bit in the mask. > > That's reasonable, but cpumask folk probably need to discuss it because > not all cpumask functions have a return value where an error may be > placed. Callers should pass sane arguments into internal functions if they expect sane output. The API not exported to userspace shouldn't sanity-check all inputs arguments. For example, cpumask_next() doesn't check srcp for NULL. However, cpumask API is exposed to drivers, and that's why optional cpumask_check() exists. (Probably. It has been done long before I took over this.) Current *generic* implementation guarantees that out-of-region offset would prevent cpumask_next() from dereferencing srcp, and makes it returning nr_cpu_ids. This behavior is expected by many callers. However, there is a couple of non-generic cpumask implementations, and one of them is written in assembler. So, the portable code shouldn't expect from cpumasks more than documentation said: for a _valid_ offset cpumask_next() returns next set bit or >= nr_cpu_ids. cpumask_check() has been broken for years. Attempting to fix it faced so much resistance, that I had to revert the patch. Now there's ongoing discussion whether we need this check at all. My opinion is that if all implementations of cpumask (more precisely, underlying bitmap API) are safe against out-of-range offset, we can simply remove cpumask_check(). Those users, like cpuinfo, who waste time on useless last iteration will bear it themselves. Thanks, Yury _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 81B4BC43217 for ; Thu, 3 Nov 2022 16:32:23 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4N38Rn6mCrz3cNJ for ; Fri, 4 Nov 2022 03:32:21 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=Jxprr5Yb; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::c31; helo=mail-oo1-xc31.google.com; envelope-from=yury.norov@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=Jxprr5Yb; dkim-atps=neutral Received: from mail-oo1-xc31.google.com (mail-oo1-xc31.google.com [IPv6:2607:f8b0:4864:20::c31]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4N38QF1TPZz3c81 for ; Fri, 4 Nov 2022 03:31:00 +1100 (AEDT) Received: by mail-oo1-xc31.google.com with SMTP id s1-20020a4a81c1000000b0047d5e28cdc0so328670oog.12 for ; Thu, 03 Nov 2022 09:31:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:from:to:cc:subject:date:message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=Jxprr5Yb6GFP2iMDdNYwrv+edL5lsVZ+sskHoEYasypEvTLy4Q6vhTS7qqFC2ZOJ9F 7UlwmUBWdL8ATQXLGSU2oLt77vVHPXn0OrJO9RZ4ozCxeZSFNH6CKpqcZtCvnwol4zeN srTpQmrtC9fSlFcJoeE3VBMkcRHxF4UC77PI0QPdBZOTiLk4O1EgX10WGEBp+gELT/Of f0hLEd5d1y3j2mzfUulOKKw9wLwe0rGXdFxYErQK/3o71DkASuqcP1UgrC5XDCQfhx1n GSTmaFTFIU7py0xRt57Y61rOd+D7tYEKoK35ZenJvXOnTlTZe3nUPKJMZ1OW7PKQwbP+ CarQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=Jkoxoghvz4QFppH3ZLOKNm3c5CPjBB+vqXoyNFMhXYJSH/IIf7Oz/URqTqt20GV/KI YFJXanejrpBL/9ROZtl/mR0bIYhcLo+FI2y5Kqu3mLmRaggiJyYvmyyRQbcaarbGiSzd Foe93/9r/QaFzdr2DdCN6f+pQobUv7jwdWYpj5WUmvHYwEa2rzASjtN5H0nLzVjDTuFL 7TFcwbBBQ+UbEJcWM7m6stJySlh79txghxEKM0OFciKq4hIOBFtjl/pgcxHH9cvwBF1u 5aCwI9igxyWKqHapMmNYHcakqfiBl6f09JEPQ2L5+Rju2umn7x0MdfWNRGTQA3Pmlv1H LohA== X-Gm-Message-State: ACrzQf18X57fp8nS738QYoDrUsiiWEmccs+CfsuOWS2plVllGuHn8IN5 PIVoklbbiMuO7NENy4h/pBA= X-Google-Smtp-Source: AMsMyM4XFkvxtx6hYtImOQooUBUuMwcsCe7CNpBD20J79Sc2FfnE52Rm7s0RT965HybTqZXYxmJ8ng== X-Received: by 2002:a4a:a609:0:b0:499:74ab:f10e with SMTP id e9-20020a4aa609000000b0049974abf10emr12101601oom.13.1667493056510; Thu, 03 Nov 2022 09:30:56 -0700 (PDT) Received: from localhost ([12.97.180.36]) by smtp.gmail.com with ESMTPSA id t15-20020a4a858f000000b00480e1109695sm370965ooh.10.2022.11.03.09.30.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 09:30:56 -0700 (PDT) From: yury.norov@gmail.com X-Google-Original-From: Yury Norov , To: Andrew Jones ; Date: Thu, 3 Nov 2022 09:30:54 -0700 Subject: Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning Message-ID: References: <20221031080604.6xei6c4e3ckhsvmy@kamzik> <20221031100327.r7tswmpszvs5ot5n@kamzik> <20221103125945.lrr5oxxmylwpam53@kamzik> <20221103153404.uh77nrdkowrxj6cr@kamzik> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221103153404.uh77nrdkowrxj6cr@kamzik> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jonas Bonn , linux-s390@vger.kernel.org, Alexander Gordeev , Dave Hansen , Vasily Gorbik , Heiko Carstens , x86@kernel.org, Linux Kernel Mailing List , Stefan Kristiansson , openrisc@lists.librecores.org, Ingo Molnar , Borislav Petkov , Paul Walmsley , Stafford Horne , Palmer Dabbelt , linux-riscv , "open list:LINUX FOR POWERPC PA SEMI PWRFICIENT" , Thomas Gleixner , Albert Ou Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Nov 03, 2022 at 04:34:04PM +0100, Andrew Jones wrote: > On Thu, Nov 03, 2022 at 04:02:12PM +0100, Borislav Petkov wrote: > > On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > > > The patch I'm proposing ensures cpumask_next()'s range, which is actually > > > [-1, nr_cpus_ids - 1), > > > > Lemme make sure I understand it correctly: on the upper boundary, if you > > supply for n the value nr_cpu_ids - 2, then it will return potentially > > the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). > > > > If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no > > further bits set. > > > > Yes, no? > > Yes > > > > > > I'll send a v4 with another stab at the commit message. > > > > Yes, and it is still an unreadable mess: "A kernel compiled with commit > > ... but not its revert... " Nope. > > > > First make sure cpumask_next()'s valid accepted range has been settled > > upon, has been explicitly documented in a comment above it and then I'll > > take a patch that fixes whatever is there to fix. > > That's fair, but I'll leave that to Yury. I'll take care of it. > > Callers should not have to filter values before passing them in - the > > function either returns an error or returns the next bit in the mask. > > That's reasonable, but cpumask folk probably need to discuss it because > not all cpumask functions have a return value where an error may be > placed. Callers should pass sane arguments into internal functions if they expect sane output. The API not exported to userspace shouldn't sanity-check all inputs arguments. For example, cpumask_next() doesn't check srcp for NULL. However, cpumask API is exposed to drivers, and that's why optional cpumask_check() exists. (Probably. It has been done long before I took over this.) Current *generic* implementation guarantees that out-of-region offset would prevent cpumask_next() from dereferencing srcp, and makes it returning nr_cpu_ids. This behavior is expected by many callers. However, there is a couple of non-generic cpumask implementations, and one of them is written in assembler. So, the portable code shouldn't expect from cpumasks more than documentation said: for a _valid_ offset cpumask_next() returns next set bit or >= nr_cpu_ids. cpumask_check() has been broken for years. Attempting to fix it faced so much resistance, that I had to revert the patch. Now there's ongoing discussion whether we need this check at all. My opinion is that if all implementations of cpumask (more precisely, underlying bitmap API) are safe against out-of-range offset, we can simply remove cpumask_check(). Those users, like cpuinfo, who waste time on useless last iteration will bear it themselves. Thanks, Yury 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 mail.librecores.org (lists.librecores.org [88.198.125.70]) by smtp.lore.kernel.org (Postfix) with ESMTP id D66CEC43217 for ; Sun, 6 Nov 2022 21:19:36 +0000 (UTC) Received: from [172.31.1.100] (localhost.localdomain [127.0.0.1]) by mail.librecores.org (Postfix) with ESMTP id 8FAB920ADE; Sun, 6 Nov 2022 22:19:35 +0100 (CET) Received: from mail-oo1-f49.google.com (mail-oo1-f49.google.com [209.85.161.49]) by mail.librecores.org (Postfix) with ESMTPS id D60C62591D for ; Thu, 3 Nov 2022 17:30:57 +0100 (CET) Received: by mail-oo1-f49.google.com with SMTP id k12-20020a4ab08c000000b0049e2ab19e04so333477oon.6 for ; Thu, 03 Nov 2022 09:30:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:from:to:cc:subject:date:message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=Jxprr5Yb6GFP2iMDdNYwrv+edL5lsVZ+sskHoEYasypEvTLy4Q6vhTS7qqFC2ZOJ9F 7UlwmUBWdL8ATQXLGSU2oLt77vVHPXn0OrJO9RZ4ozCxeZSFNH6CKpqcZtCvnwol4zeN srTpQmrtC9fSlFcJoeE3VBMkcRHxF4UC77PI0QPdBZOTiLk4O1EgX10WGEBp+gELT/Of f0hLEd5d1y3j2mzfUulOKKw9wLwe0rGXdFxYErQK/3o71DkASuqcP1UgrC5XDCQfhx1n GSTmaFTFIU7py0xRt57Y61rOd+D7tYEKoK35ZenJvXOnTlTZe3nUPKJMZ1OW7PKQwbP+ CarQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=2wf8Ox+cqI36jbP+T7p6EKTUvmpF6v5KouIok7EbcyU=; b=GjQ6uVrbiWwe9pnCYa8Y2mumVcvdf+uEm2ZMU51SYqfLlCHkkHLGIqXLXqVkFeiI8F g2UzRPC9NkIwGgPqOIP8B9KVOyrv3p7HLfe3vw69KrJi7Xt+5vWTFHoF3jnVkRU3t0Zm CC/gd8DopjSjsK4/E6xb6cpYQhZwW38rrXl3FCGfoHg1MZ0xr0p3B8UsFzS3buzlDTvi VQehHnpajho5QczK/psCH3qJECcKVXtDewVPF86APIRkGHslsgIpktWsmh61+ZKnCkk3 V8PRFTS9AsJhd+fL73stiGwn1Pikgy/DSvk7qYAxbY6vD0Mh3I1l4MuRHpTNtMkgp75N mMhQ== X-Gm-Message-State: ACrzQf1cK4C6GHFY0RHlfb0Xa0/Xpu6ixmeAWBAR15DrdESpSq4cEWpa aODtby6woan86rhXvZGz5yE= X-Google-Smtp-Source: AMsMyM4XFkvxtx6hYtImOQooUBUuMwcsCe7CNpBD20J79Sc2FfnE52Rm7s0RT965HybTqZXYxmJ8ng== X-Received: by 2002:a4a:a609:0:b0:499:74ab:f10e with SMTP id e9-20020a4aa609000000b0049974abf10emr12101601oom.13.1667493056510; Thu, 03 Nov 2022 09:30:56 -0700 (PDT) Received: from localhost ([12.97.180.36]) by smtp.gmail.com with ESMTPSA id t15-20020a4a858f000000b00480e1109695sm370965ooh.10.2022.11.03.09.30.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Nov 2022 09:30:56 -0700 (PDT) From: yury.norov@gmail.com X-Google-Original-From: Yury Norov , To: Andrew Jones ; Date: Thu, 3 Nov 2022 09:30:54 -0700 Subject: Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning Message-ID: References: <20221031080604.6xei6c4e3ckhsvmy@kamzik> <20221031100327.r7tswmpszvs5ot5n@kamzik> <20221103125945.lrr5oxxmylwpam53@kamzik> <20221103153404.uh77nrdkowrxj6cr@kamzik> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221103153404.uh77nrdkowrxj6cr@kamzik> X-Mailman-Approved-At: Sun, 06 Nov 2022 22:19:32 +0100 X-BeenThere: openrisc@lists.librecores.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: Discussion around the OpenRISC processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jonas Bonn , linux-s390@vger.kernel.org, Alexander Gordeev , Dave Hansen , Vasily Gorbik , Michael Ellerman , Heiko Carstens , x86@kernel.org, Linux Kernel Mailing List , openrisc@lists.librecores.org, Ingo Molnar , Borislav Petkov , Paul Walmsley , Palmer Dabbelt , linux-riscv , "open list:LINUX FOR POWERPC PA SEMI PWRFICIENT" , Thomas Gleixner , Albert Ou Errors-To: openrisc-bounces@lists.librecores.org Sender: "OpenRISC" On Thu, Nov 03, 2022 at 04:34:04PM +0100, Andrew Jones wrote: > On Thu, Nov 03, 2022 at 04:02:12PM +0100, Borislav Petkov wrote: > > On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > > > The patch I'm proposing ensures cpumask_next()'s range, which is actually > > > [-1, nr_cpus_ids - 1), > > > > Lemme make sure I understand it correctly: on the upper boundary, if you > > supply for n the value nr_cpu_ids - 2, then it will return potentially > > the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). > > > > If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no > > further bits set. > > > > Yes, no? > > Yes > > > > > > I'll send a v4 with another stab at the commit message. > > > > Yes, and it is still an unreadable mess: "A kernel compiled with commit > > ... but not its revert... " Nope. > > > > First make sure cpumask_next()'s valid accepted range has been settled > > upon, has been explicitly documented in a comment above it and then I'll > > take a patch that fixes whatever is there to fix. > > That's fair, but I'll leave that to Yury. I'll take care of it. > > Callers should not have to filter values before passing them in - the > > function either returns an error or returns the next bit in the mask. > > That's reasonable, but cpumask folk probably need to discuss it because > not all cpumask functions have a return value where an error may be > placed. Callers should pass sane arguments into internal functions if they expect sane output. The API not exported to userspace shouldn't sanity-check all inputs arguments. For example, cpumask_next() doesn't check srcp for NULL. However, cpumask API is exposed to drivers, and that's why optional cpumask_check() exists. (Probably. It has been done long before I took over this.) Current *generic* implementation guarantees that out-of-region offset would prevent cpumask_next() from dereferencing srcp, and makes it returning nr_cpu_ids. This behavior is expected by many callers. However, there is a couple of non-generic cpumask implementations, and one of them is written in assembler. So, the portable code shouldn't expect from cpumasks more than documentation said: for a _valid_ offset cpumask_next() returns next set bit or >= nr_cpu_ids. cpumask_check() has been broken for years. Attempting to fix it faced so much resistance, that I had to revert the patch. Now there's ongoing discussion whether we need this check at all. My opinion is that if all implementations of cpumask (more precisely, underlying bitmap API) are safe against out-of-range offset, we can simply remove cpumask_check(). Those users, like cpuinfo, who waste time on useless last iteration will bear it themselves. Thanks, Yury