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 BA6BCC433EF for ; Sun, 10 Apr 2022 12:41:56 +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:To:References:Message-Id:Cc:Date: In-Reply-To:From:Subject:Mime-Version:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7AAJ/o8iOrkTx5ejh7DqMbenbMAooW31Cy2u34m1sWc=; b=QTb2Uab3A/PVAI PbntghI/3KKUah6aShsXlk/216J/uUsLLnXY0duyjxk2oAzUKar7eKdmu3acgLBbmshgx8Zij9RCZ BNv6T1c8lcMwYU3l67YrnDaoOVN0PC0M01WtNzQMnLvhWD67YKii+GFIoe7p9JcK0/e46byB83dO0 Nm2YCZTPyJdMWrzSseSCGeWueB4obhvnUAwUhZJcr1gqC4sPliKYdCmNPuTqKq0dlI01YbqSyVExf nAsOZPJtWvL0/Ulye+K82ResFkO35dIgeW+AKJbWuFNHO7lA9GC1I2g2LCxE+FBgkHfJXyqZIYB96 hzi9o5IiU2siOkfQag/w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ndWrS-004rGY-QQ; Sun, 10 Apr 2022 12:40:10 +0000 Received: from mail-ej1-x62f.google.com ([2a00:1450:4864:20::62f]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ndWrP-004rG3-3f for linux-arm-kernel@lists.infradead.org; Sun, 10 Apr 2022 12:40:08 +0000 Received: by mail-ej1-x62f.google.com with SMTP id ks6so1083118ejb.1 for ; Sun, 10 Apr 2022 05:40:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=WJ3zx5hxbrhQdoh4df/mkYbAfZTieLScs1fGc8yvAKE=; b=genybavvxQfO3oMBZjo2CjIk1O/oGwGjSShnuF7VRUqPU857l7tmI7mOpqspRCdNa8 PP0OIXI2vNtXcK+auG9ZXMTFfhBJFd0oUTv8b4pdD3v7PeUC32Ge/N4RYYU9PNXTmgJE zl+60cT3783cP9JX4TQBR8pW957rVNarSQlgjCj2xNVl4mqDV72XT3VCx+sVT9VZ1DHg zQaUavreMRULymshsvHLE8NMNW7w3nEqeO1w59qryQUYbxeUYgeVZGqKcEuDOH+/Mstm /dypm4G0EB4d+XBiyN7xyFepVohVcq2zwf8oZ9d6urnWhkNoNMDQ1bECwpDFSUHNF7wj Nq1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=WJ3zx5hxbrhQdoh4df/mkYbAfZTieLScs1fGc8yvAKE=; b=tmcy+9PbBxtlSrPuy+add11eIa+5I0N/4RKSgKtCexHZdxy6pXbtdolyn0EwersSha FjjADN9ddRFp9rEwXV7Fjv2qPb5+Y8k1KYabZ+xynn5YWSiWh8zsSea/rO/US9eb56CK DR7hL7sRSIsCC+auIbZx6x06Ol4Eksm3SG5mNUsg5rLp5pf8+1t+LNXoX5UqXnwOCNaX rN3Pasci177trmlkRArLWz4WboCPmCBY+ZZBRU/GXW+XWuKNYBqwYEkzLtWwePY+KPZZ Z2732kd1PmNNZha0iwJGERs7aQs6D8QVE8MdVZZGqmlpkzNzjpRH2uPZ5j7e8ECIqRDz SJ9Q== X-Gm-Message-State: AOAM531dJMnTy97oNi1YU0IpNwJ2mbFilpUN7zJiClqdy95jRNeQJ4no Lq+shxbocFd0euUz7K1JV9U= X-Google-Smtp-Source: ABdhPJyyaBS7UlkYvO3wuO5S0f8bGGkJ6uVCS0ltBvj9Hg44UfZ+IoAoo+jvga3Jvjur3dumJsVFrg== X-Received: by 2002:a17:906:7943:b0:6df:e5b3:6553 with SMTP id l3-20020a170906794300b006dfe5b36553mr25955888ejo.398.1649594404580; Sun, 10 Apr 2022 05:40:04 -0700 (PDT) Received: from smtpclient.apple (ip-185-104-137-32.ptr.icomera.net. [185.104.137.32]) by smtp.gmail.com with ESMTPSA id tj13-20020a170907c24d00b006e853804a70sm2598630ejc.0.2022.04.10.05.39.52 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 10 Apr 2022 05:40:04 -0700 (PDT) Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.80.82.1.1\)) Subject: Re: [PATCH net-next 02/15] net: dsa: sja1105: Remove usage of iterator for list_add() after loop From: Jakob Koschel In-Reply-To: <20220410110508.em3r7z62ufqcbrfm@skbuf> Date: Sun, 10 Apr 2022 14:39:48 +0200 Cc: "David S. Miller" , Jakub Kicinski , Paolo Abeni , Andrew Lunn , Vivien Didelot , Florian Fainelli , Lars Povlsen , Steen Hegelund , UNGLinuxDriver@microchip.com, Ariel Elior , Manish Chopra , Edward Cree , Martin Habets , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Jiri Pirko , Casper Andersson , Bjarni Jonasson , Colin Ian King , Michael Walle , Christophe JAILLET , Arnd Bergmann , Eric Dumazet , Di Zhu , Xu Wang , Netdev , LKML , Linux ARM , linuxppc-dev , Mike Rapoport , Brian Johannesmeyer , Cristiano Giuffrida , "Bos, H.J." Message-Id: <935062D0-C657-4C79-A0BE-70141D052EC0@gmail.com> References: <20220407102900.3086255-1-jakobkoschel@gmail.com> <20220407102900.3086255-3-jakobkoschel@gmail.com> <20220408114120.tvf2lxvhfqbnrlml@skbuf> <20220410110508.em3r7z62ufqcbrfm@skbuf> To: Vladimir Oltean X-Mailer: Apple Mail (2.3696.80.82.1.1) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220410_054007_215223_3F4F34E5 X-CRM114-Status: GOOD ( 29.00 ) X-BeenThere: linux-arm-kernel@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-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > On 10. Apr 2022, at 13:05, Vladimir Oltean wrote: > > On Sun, Apr 10, 2022 at 12:51:56PM +0200, Jakob Koschel wrote: >> I've just looked at this again in a bit more detail while integrating it into the patch series. >> >> I realized that this just shifts the 'problem' to using the 'pos' iterator variable after the loop. >> If the scope of the list iterator would be lowered to the list traversal loop it would also make sense >> to also do it for list_for_each(). > > Yes, but list_for_each() was never formulated as being problematic in > the same way as list_for_each_entry(), was it? I guess I'm starting to > not understand what is the true purpose of the changes. Sorry for having caused the confusion. Let me elaborate a bit to give more context. There are two main benefits of this entire effort. 1) fix the architectural bugs and avoid any missuse of the list iterator after the loop by construction. This only concerns the list_for_each_entry_*() macros and your change will allow lowering the scope for all of those. It might be debatable that it would be more consistent to lower the scope for list_for_each() as well, but it wouldn't be strictly necessary. 2) fix *possible* speculative bugs. In our project, Kasper [1], we were able to show that this can be an issue for the list traversal macros (and this is how the entire effort started). The reason is that the processor might run an additional loop iteration in speculative execution with the iterator variable computed based on the head element. This can (and we have verified this) happen if the CPU incorrectly assumes !list_entry_is_head(pos, head, member). If this happens, all memory accesses based on the iterator variable *potentially* open the chance for spectre [2] gadgets. The proposed mitigation was setting the iterator variable to NULL when the terminating condition is reached (in speculative safe way). Then, the additional speculative list iteration would still execute but won't access any potential secret data. And this would also be required for list_for_each() since combined with the list_entry() within the loop it basically is semantically identical to list_for_each_entry() for the additional speculative iteration. Now, I have no strong opinion on going all the way and since 2) is not the main motivation for this I'm also fine with sticking to your proposed solution, but it would mean that implementing a "speculative safe" list_for_each() will be more difficult in the future since it is using the iterator of list_for_each() past the loop. I hope this explains the background a bit better. > >> What do you think about doing it this way: >> >> diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c >> index b7e95d60a6e4..f5b0502c1098 100644 >> --- a/drivers/net/dsa/sja1105/sja1105_vl.c >> +++ b/drivers/net/dsa/sja1105/sja1105_vl.c >> @@ -28,6 +28,7 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg, >> list_add(&e->list, &gating_cfg->entries); >> } else { >> struct sja1105_gate_entry *p; >> + struct list_head *pos = NULL; >> >> list_for_each_entry(p, &gating_cfg->entries, list) { >> if (p->interval == e->interval) { >> @@ -37,10 +38,14 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg, >> goto err; >> } >> >> - if (e->interval < p->interval) >> + if (e->interval < p->interval) { >> + pos = &p->list; >> break; >> + } >> } >> - list_add(&e->list, p->list.prev); >> + if (!pos) >> + pos = &gating_cfg->entries; >> + list_add(&e->list, pos->prev); >> } >> >> gating_cfg->num_entries++; >> -- >> >>> >>> Thanks for the suggestion. >>> >>>> } >>>> >>>> gating_cfg->num_entries++; >>>> -----------------------------[ cut here ]----------------------------- >>> >>> [1] https://lore.kernel.org/linux-kernel/20220407102900.3086255-12-jakobkoschel@gmail.com/ >>> >>> Jakob >> >> Thanks, >> Jakob Thanks, Jakob [1] https://www.vusec.net/projects/kasper/ [2] https://spectreattack.com/spectre.pdf _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel