From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bedivere.hansenpartnership.com (bedivere.hansenpartnership.com [96.44.175.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54DFC15BF for ; Mon, 28 Feb 2022 20:43:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Cc: Jakob Koschel , alsa-devel@alsa-project.org, linux-aspeed@lists.ozlabs.org, "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , amd-gfx list , samba-technical@lists.samba.org, linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , KVM list , linux-scsi , linux-rdma , linux-staging@lists.linux.dev, "Bos, H.J." , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , linux-fsdevel , Christophe JAILLET , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, linux-wireless , Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , dma , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which > > requires > > that "-std=gnu11" that was discussed in the original thread). > > > > That will guarantee that the 'pos' parameter of > > list_for_each_entry() > > is only updated INSIDE the for_each_list_entry() loop, and can > > never > > point to the (wrongly typed) head entry. > > > > And I would actually hope that it should actually cause compiler > > warnings about possibly uninitialized variables if people then use > > the > > 'pos' pointer outside the loop. Except > > > > (a) that code in sgx/encl.c currently initializes 'tmp' to NULL > > for > > inexplicable reasons - possibly because it already expected this > > behavior > > > > (b) when I remove that NULL initializer, I still don't get a > > warning, > > because we've disabled -Wno-maybe-uninitialized since it results in > > so > > many false positives. > > > > Oh well. > > > > Anyway, give this patch a look, and at least if it's expanded to do > > "(pos) = NULL" in the entry statement for the for-loop, it will > > avoid the HEAD type confusion that Jakob is working on. And I think > > in a cleaner way than the horrid games he plays. > > > > (But it won't avoid possible CPU speculation of such type > > confusion. That, in my opinion, is a completely different issue) > > Yes, completely agree. > > > I do wish we could actually poison the 'pos' value after the loop > > somehow - but clearly the "might be uninitialized" I was hoping for > > isn't the way to do it. > > > > Anybody have any ideas? > > I think we should look at the use cases why code is touching (pos) > after the loop. > > Just from skimming over the patches to change this and experience > with the drivers/subsystems I help to maintain I think the primary > pattern looks something like this: > > list_for_each_entry(entry, head, member) { > if (some_condition_checking(entry)) > break; > } > do_something_with(entry); Actually, we usually have a check to see if the loop found anything, but in that case it should something like if (list_entry_is_head(entry, head, member)) { return with error; } do_somethin_with(entry); Suffice? The list_entry_is_head() macro is designed to cope with the bogus entry on head problem. James 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 C3439C433F5 for ; Mon, 28 Feb 2022 20:43:14 +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:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=4H2EWyAiyipmSF23K8C4yw+W0bzVymgMlgoVnGLQ/DA=; b=r9KrP9c81XeWKJ EO2//Piq2I6lVSy/su7MEM/Xnr0EwERruJhBctO7pzRyRqnYeiO5WCz1zHJy0jfdlVZieNsGAfy4c h5x9QRVOzNRferc8M1CN0PoV9Ht276qKeQtyIh82tHuXCMvJLPzZSROaO0C7zKmMYv30CBJisFqoJ diLOBq7SbmiYY5B/Vl6MmX62PCaejxSj6DGmCRCcc+gVWNEdCfpk/jqZIH7kFdzCfdrH57CaQbBAV p/0N+NEtJFZFQW5hbuEfunCcEEQL4gHgbzB3c8IIQG942KQLOZjfTgXBl6j/GSEaVWO4xI1E1uhrR +0ZqsSKaNGNnxM54OAKA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nOmrL-00E3jJ-8E; Mon, 28 Feb 2022 20:43:07 +0000 Received: from bedivere.hansenpartnership.com ([96.44.175.130]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nOmrI-00E3hd-2I; Mon, 28 Feb 2022 20:43:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Cc: Jakob Koschel , alsa-devel@alsa-project.org, linux-aspeed@lists.ozlabs.org, "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , amd-gfx list , samba-technical@lists.samba.org, linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , KVM list , linux-scsi , linux-rdma , linux-staging@lists.linux.dev, "Bos, H.J." , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , linux-fsdevel , Christophe JAILLET , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, linux-wireless , Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , dma , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> User-Agent: Evolution 3.34.4 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220228_124304_127441_84E585AC X-CRM114-Status: GOOD ( 36.90 ) X-BeenThere: linux-mediatek@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="utf-8" Content-Transfer-Encoding: base64 Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org T24gTW9uLCAyMDIyLTAyLTI4IGF0IDIxOjA3ICswMTAwLCBDaHJpc3RpYW4gS8O2bmlnIHdyb3Rl Ogo+IEFtIDI4LjAyLjIyIHVtIDIwOjU2IHNjaHJpZWIgTGludXMgVG9ydmFsZHM6Cj4gPiBPbiBN b24sIEZlYiAyOCwgMjAyMiBhdCA0OjE5IEFNIENocmlzdGlhbiBLw7ZuaWcKPiA+IDxjaHJpc3Rp YW4ua29lbmlnQGFtZC5jb20+IHdyb3RlOgo+ID4gPiBJIGRvbid0IHRoaW5rIHRoYXQgdXNpbmcg dGhlIGV4dHJhIHZhcmlhYmxlIG1ha2VzIHRoZSBjb2RlIGluIGFueQo+ID4gPiB3YXkKPiA+ID4g bW9yZSByZWxpYWJsZSBvciBlYXNpZXIgdG8gcmVhZC4KPiA+IFNvIEkgdGhpbmsgdGhlIG5leHQg c3RlcCBpcyB0byBkbyB0aGUgYXR0YWNoZWQgcGF0Y2ggKHdoaWNoCj4gPiByZXF1aXJlcwo+ID4g dGhhdCAiLXN0ZD1nbnUxMSIgdGhhdCB3YXMgZGlzY3Vzc2VkIGluIHRoZSBvcmlnaW5hbCB0aHJl YWQpLgo+ID4gCj4gPiBUaGF0IHdpbGwgZ3VhcmFudGVlIHRoYXQgdGhlICdwb3MnIHBhcmFtZXRl ciBvZgo+ID4gbGlzdF9mb3JfZWFjaF9lbnRyeSgpCj4gPiBpcyBvbmx5IHVwZGF0ZWQgSU5TSURF IHRoZSBmb3JfZWFjaF9saXN0X2VudHJ5KCkgbG9vcCwgYW5kIGNhbgo+ID4gbmV2ZXIKPiA+IHBv aW50IHRvIHRoZSAod3JvbmdseSB0eXBlZCkgaGVhZCBlbnRyeS4KPiA+IAo+ID4gQW5kIEkgd291 bGQgYWN0dWFsbHkgaG9wZSB0aGF0IGl0IHNob3VsZCBhY3R1YWxseSBjYXVzZSBjb21waWxlcgo+ ID4gd2FybmluZ3MgYWJvdXQgcG9zc2libHkgdW5pbml0aWFsaXplZCB2YXJpYWJsZXMgaWYgcGVv cGxlIHRoZW4gdXNlCj4gPiB0aGUKPiA+ICdwb3MnIHBvaW50ZXIgb3V0c2lkZSB0aGUgbG9vcC4g RXhjZXB0Cj4gPiAKPiA+ICAgKGEpIHRoYXQgY29kZSBpbiBzZ3gvZW5jbC5jIGN1cnJlbnRseSBp bml0aWFsaXplcyAndG1wJyB0byBOVUxMCj4gPiBmb3IKPiA+IGluZXhwbGljYWJsZSByZWFzb25z IC0gcG9zc2libHkgYmVjYXVzZSBpdCBhbHJlYWR5IGV4cGVjdGVkIHRoaXMKPiA+IGJlaGF2aW9y Cj4gPiAKPiA+ICAgKGIpIHdoZW4gSSByZW1vdmUgdGhhdCBOVUxMIGluaXRpYWxpemVyLCBJIHN0 aWxsIGRvbid0IGdldCBhCj4gPiB3YXJuaW5nLAo+ID4gYmVjYXVzZSB3ZSd2ZSBkaXNhYmxlZCAt V25vLW1heWJlLXVuaW5pdGlhbGl6ZWQgc2luY2UgaXQgcmVzdWx0cyBpbgo+ID4gc28KPiA+IG1h bnkgZmFsc2UgcG9zaXRpdmVzLgo+ID4gCj4gPiBPaCB3ZWxsLgo+ID4gCj4gPiBBbnl3YXksIGdp dmUgdGhpcyBwYXRjaCBhIGxvb2ssIGFuZCBhdCBsZWFzdCBpZiBpdCdzIGV4cGFuZGVkIHRvIGRv Cj4gPiAiKHBvcykgPSBOVUxMIiBpbiB0aGUgZW50cnkgc3RhdGVtZW50IGZvciB0aGUgZm9yLWxv b3AsIGl0IHdpbGwKPiA+IGF2b2lkIHRoZSBIRUFEIHR5cGUgY29uZnVzaW9uIHRoYXQgSmFrb2Ig aXMgd29ya2luZyBvbi4gQW5kIEkgdGhpbmsKPiA+IGluIGEgY2xlYW5lciB3YXkgdGhhbiB0aGUg aG9ycmlkIGdhbWVzIGhlIHBsYXlzLgo+ID4gCj4gPiAoQnV0IGl0IHdvbid0IGF2b2lkIHBvc3Np YmxlIENQVSBzcGVjdWxhdGlvbiBvZiBzdWNoIHR5cGUKPiA+IGNvbmZ1c2lvbi4gVGhhdCwgaW4g bXkgb3BpbmlvbiwgaXMgYSBjb21wbGV0ZWx5IGRpZmZlcmVudCBpc3N1ZSkKPiAKPiBZZXMsIGNv bXBsZXRlbHkgYWdyZWUuCj4gCj4gPiBJIGRvIHdpc2ggd2UgY291bGQgYWN0dWFsbHkgcG9pc29u IHRoZSAncG9zJyB2YWx1ZSBhZnRlciB0aGUgbG9vcAo+ID4gc29tZWhvdyAtIGJ1dCBjbGVhcmx5 IHRoZSAibWlnaHQgYmUgdW5pbml0aWFsaXplZCIgSSB3YXMgaG9waW5nIGZvcgo+ID4gaXNuJ3Qg dGhlIHdheSB0byBkbyBpdC4KPiA+IAo+ID4gQW55Ym9keSBoYXZlIGFueSBpZGVhcz8KPiAKPiBJ IHRoaW5rIHdlIHNob3VsZCBsb29rIGF0IHRoZSB1c2UgY2FzZXMgd2h5IGNvZGUgaXMgdG91Y2hp bmcgKHBvcykKPiBhZnRlciB0aGUgbG9vcC4KPiAKPiBKdXN0IGZyb20gc2tpbW1pbmcgb3ZlciB0 aGUgcGF0Y2hlcyB0byBjaGFuZ2UgdGhpcyBhbmQgZXhwZXJpZW5jZQo+IHdpdGggdGhlIGRyaXZl cnMvc3Vic3lzdGVtcyBJIGhlbHAgdG8gbWFpbnRhaW4gSSB0aGluayB0aGUgcHJpbWFyeQo+IHBh dHRlcm4gbG9va3Mgc29tZXRoaW5nIGxpa2UgdGhpczoKPiAKPiBsaXN0X2Zvcl9lYWNoX2VudHJ5 KGVudHJ5LCBoZWFkLCBtZW1iZXIpIHsKPiAgICAgIGlmIChzb21lX2NvbmRpdGlvbl9jaGVja2lu ZyhlbnRyeSkpCj4gICAgICAgICAgYnJlYWs7Cj4gfQo+IGRvX3NvbWV0aGluZ193aXRoKGVudHJ5 KTsKCgpBY3R1YWxseSwgd2UgdXN1YWxseSBoYXZlIGEgY2hlY2sgdG8gc2VlIGlmIHRoZSBsb29w IGZvdW5kIGFueXRoaW5nLApidXQgaW4gdGhhdCBjYXNlIGl0IHNob3VsZCBzb21ldGhpbmcgbGlr ZQoKaWYgKGxpc3RfZW50cnlfaXNfaGVhZChlbnRyeSwgaGVhZCwgbWVtYmVyKSkgewogICAgcmV0 dXJuIHdpdGggZXJyb3I7Cn0KZG9fc29tZXRoaW5fd2l0aChlbnRyeSk7CgpTdWZmaWNlPyAgVGhl IGxpc3RfZW50cnlfaXNfaGVhZCgpIG1hY3JvIGlzIGRlc2lnbmVkIHRvIGNvcGUgd2l0aCB0aGUK Ym9ndXMgZW50cnkgb24gaGVhZCBwcm9ibGVtLgoKSmFtZXMKCgoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXgtbWVkaWF0ZWsgbWFpbGluZyBsaXN0 CkxpbnV4LW1lZGlhdGVrQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVh ZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1tZWRpYXRlawo= 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 alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (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 28153C433FE for ; Wed, 2 Mar 2022 08:48:10 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 571721B49; Wed, 2 Mar 2022 09:47:18 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 571721B49 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646210888; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Subject:From:To:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=CHb3FwIpX+5viLa/7jR9Tr/BRWPAy/iy4sS5qqHu86DHwqX/AGf14LQMqgVe4Vnwu Nzpn/Vem2h6A7+xPEg7KcHAxNr0AyNkYdD+zpgbCUyEyhwy5Irzo6cdQ5Vmggbc8ID E33AbJaZn0VsVbWjpAKX07ux/3hxhvBJboOUXR/8= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 75E00F80637; Wed, 2 Mar 2022 09:34:08 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 508EEF8013C; Mon, 28 Feb 2022 21:43:12 +0100 (CET) Received: from bedivere.hansenpartnership.com (bedivere.hansenpartnership.com [IPv6:2607:fcd0:100:8a00::2]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 1E7B5F80054 for ; Mon, 28 Feb 2022 21:43:03 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 1E7B5F80054 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=hansenpartnership.com header.i=@hansenpartnership.com header.b="ujxiIgy0"; dkim=pass (1024-bit key) header.d=hansenpartnership.com header.i=@hansenpartnership.com header.b="ujxiIgy0" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Wed, 02 Mar 2022 09:33:33 +0100 Cc: linux-wireless , alsa-devel@alsa-project.org, KVM list , "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , "Bos, H.J." , linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , linux-aspeed@lists.ozlabs.org, linux-scsi , linux-rdma , linux-staging@lists.linux.dev, amd-gfx list , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , dma , Christophe JAILLET , Jakob Koschel , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, samba-technical@lists.samba.org, Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , linux-fsdevel , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which > > requires > > that "-std=gnu11" that was discussed in the original thread). > > > > That will guarantee that the 'pos' parameter of > > list_for_each_entry() > > is only updated INSIDE the for_each_list_entry() loop, and can > > never > > point to the (wrongly typed) head entry. > > > > And I would actually hope that it should actually cause compiler > > warnings about possibly uninitialized variables if people then use > > the > > 'pos' pointer outside the loop. Except > > > > (a) that code in sgx/encl.c currently initializes 'tmp' to NULL > > for > > inexplicable reasons - possibly because it already expected this > > behavior > > > > (b) when I remove that NULL initializer, I still don't get a > > warning, > > because we've disabled -Wno-maybe-uninitialized since it results in > > so > > many false positives. > > > > Oh well. > > > > Anyway, give this patch a look, and at least if it's expanded to do > > "(pos) = NULL" in the entry statement for the for-loop, it will > > avoid the HEAD type confusion that Jakob is working on. And I think > > in a cleaner way than the horrid games he plays. > > > > (But it won't avoid possible CPU speculation of such type > > confusion. That, in my opinion, is a completely different issue) > > Yes, completely agree. > > > I do wish we could actually poison the 'pos' value after the loop > > somehow - but clearly the "might be uninitialized" I was hoping for > > isn't the way to do it. > > > > Anybody have any ideas? > > I think we should look at the use cases why code is touching (pos) > after the loop. > > Just from skimming over the patches to change this and experience > with the drivers/subsystems I help to maintain I think the primary > pattern looks something like this: > > list_for_each_entry(entry, head, member) { > if (some_condition_checking(entry)) > break; > } > do_something_with(entry); Actually, we usually have a check to see if the loop found anything, but in that case it should something like if (list_entry_is_head(entry, head, member)) { return with error; } do_somethin_with(entry); Suffice? The list_entry_is_head() macro is designed to cope with the bogus entry on head problem. James 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 2C073C433EF for ; Mon, 28 Feb 2022 20:53:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9584010E442; Mon, 28 Feb 2022 20:53:24 +0000 (UTC) X-Greylist: delayed 623 seconds by postgrey-1.36 at gabe; Mon, 28 Feb 2022 20:53:23 UTC Received: from bedivere.hansenpartnership.com (bedivere.hansenpartnership.com [IPv6:2607:fcd0:100:8a00::2]) by gabe.freedesktop.org (Postfix) with ESMTPS id F408F10E442; Mon, 28 Feb 2022 20:53:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-wireless , alsa-devel@alsa-project.org, KVM list , "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , "Bos, H.J." , linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , linux-aspeed@lists.ozlabs.org, linux-scsi , linux-rdma , linux-staging@lists.linux.dev, amd-gfx list , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , dma , Christophe JAILLET , Jakob Koschel , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, samba-technical@lists.samba.org, Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , linux-fsdevel , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which > > requires > > that "-std=gnu11" that was discussed in the original thread). > > > > That will guarantee that the 'pos' parameter of > > list_for_each_entry() > > is only updated INSIDE the for_each_list_entry() loop, and can > > never > > point to the (wrongly typed) head entry. > > > > And I would actually hope that it should actually cause compiler > > warnings about possibly uninitialized variables if people then use > > the > > 'pos' pointer outside the loop. Except > > > > (a) that code in sgx/encl.c currently initializes 'tmp' to NULL > > for > > inexplicable reasons - possibly because it already expected this > > behavior > > > > (b) when I remove that NULL initializer, I still don't get a > > warning, > > because we've disabled -Wno-maybe-uninitialized since it results in > > so > > many false positives. > > > > Oh well. > > > > Anyway, give this patch a look, and at least if it's expanded to do > > "(pos) = NULL" in the entry statement for the for-loop, it will > > avoid the HEAD type confusion that Jakob is working on. And I think > > in a cleaner way than the horrid games he plays. > > > > (But it won't avoid possible CPU speculation of such type > > confusion. That, in my opinion, is a completely different issue) > > Yes, completely agree. > > > I do wish we could actually poison the 'pos' value after the loop > > somehow - but clearly the "might be uninitialized" I was hoping for > > isn't the way to do it. > > > > Anybody have any ideas? > > I think we should look at the use cases why code is touching (pos) > after the loop. > > Just from skimming over the patches to change this and experience > with the drivers/subsystems I help to maintain I think the primary > pattern looks something like this: > > list_for_each_entry(entry, head, member) { > if (some_condition_checking(entry)) > break; > } > do_something_with(entry); Actually, we usually have a check to see if the loop found anything, but in that case it should something like if (list_entry_is_head(entry, head, member)) { return with error; } do_somethin_with(entry); Suffice? The list_entry_is_head() macro is designed to cope with the bogus entry on head problem. James 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.sourceforge.net (lists.sourceforge.net [216.105.38.7]) (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 05AF6C433EF for ; Mon, 28 Feb 2022 21:08:36 +0000 (UTC) Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.94.2) (envelope-from ) id 1nOnG1-0005Ri-2C; Mon, 28 Feb 2022 21:08:35 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nOnFx-0005RL-Rs; Mon, 28 Feb 2022 21:08:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:Content-Type :References:In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=TDrjWGoS9dlu/1fHoeSyRYjbIFPec7jTIRG3GcE4fUY=; b=JYtcBnPcqMhjiYCsB0+cna+F2/ dRhgCaIr2U0RwYd9V8nvWgZvQoMfv6gLx58YF0VB2vkGUR9sibl7DK8DDUEUZ6LxAz2HQZIwHrI0L FQmBShye5GDYCO4aoEOAqSTK7Zbf8Wm5OW1rrPpc6gHzZlOtpxekTbJw+Hz+Mbzri2tk=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References: In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=TDrjWGoS9dlu/1fHoeSyRYjbIFPec7jTIRG3GcE4fUY=; b=Wxvk3bfdpR2qNzdp+hhxNWYoMY m8Wd3DW0swcaHLMtgBsEjdRHbzPBUFbr+xQNlfT9UISQ+QKh44A5uKoZhbKUQf41iXWW8f9FOq0zD zmknbg321j1PTpDnrT/x5+RY2kFjm3rIE88ZrxxSFiKiiObQhHiwQcSSIVyWvTMm0U3k=; Received: from bedivere.hansenpartnership.com ([96.44.175.130]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.94.2) id 1nOnFs-000VdT-P6; Mon, 28 Feb 2022 21:08:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> User-Agent: Evolution 3.34.4 MIME-Version: 1.0 X-Headers-End: 1nOnFs-000VdT-P6 Subject: Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr X-BeenThere: linux-f2fs-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-wireless , alsa-devel@alsa-project.org, KVM list , "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , "Bos, H.J." , linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , linux-aspeed@lists.ozlabs.org, linux-scsi , linux-rdma , linux-staging@lists.linux.dev, amd-gfx list , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , dma , Christophe JAILLET , Jakob Koschel , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, samba-technical@lists.samba.org, Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , linux-fsdevel , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net T24gTW9uLCAyMDIyLTAyLTI4IGF0IDIxOjA3ICswMTAwLCBDaHJpc3RpYW4gS8O2bmlnIHdyb3Rl Ogo+IEFtIDI4LjAyLjIyIHVtIDIwOjU2IHNjaHJpZWIgTGludXMgVG9ydmFsZHM6Cj4gPiBPbiBN b24sIEZlYiAyOCwgMjAyMiBhdCA0OjE5IEFNIENocmlzdGlhbiBLw7ZuaWcKPiA+IDxjaHJpc3Rp YW4ua29lbmlnQGFtZC5jb20+IHdyb3RlOgo+ID4gPiBJIGRvbid0IHRoaW5rIHRoYXQgdXNpbmcg dGhlIGV4dHJhIHZhcmlhYmxlIG1ha2VzIHRoZSBjb2RlIGluIGFueQo+ID4gPiB3YXkKPiA+ID4g bW9yZSByZWxpYWJsZSBvciBlYXNpZXIgdG8gcmVhZC4KPiA+IFNvIEkgdGhpbmsgdGhlIG5leHQg c3RlcCBpcyB0byBkbyB0aGUgYXR0YWNoZWQgcGF0Y2ggKHdoaWNoCj4gPiByZXF1aXJlcwo+ID4g dGhhdCAiLXN0ZD1nbnUxMSIgdGhhdCB3YXMgZGlzY3Vzc2VkIGluIHRoZSBvcmlnaW5hbCB0aHJl YWQpLgo+ID4gCj4gPiBUaGF0IHdpbGwgZ3VhcmFudGVlIHRoYXQgdGhlICdwb3MnIHBhcmFtZXRl ciBvZgo+ID4gbGlzdF9mb3JfZWFjaF9lbnRyeSgpCj4gPiBpcyBvbmx5IHVwZGF0ZWQgSU5TSURF IHRoZSBmb3JfZWFjaF9saXN0X2VudHJ5KCkgbG9vcCwgYW5kIGNhbgo+ID4gbmV2ZXIKPiA+IHBv aW50IHRvIHRoZSAod3JvbmdseSB0eXBlZCkgaGVhZCBlbnRyeS4KPiA+IAo+ID4gQW5kIEkgd291 bGQgYWN0dWFsbHkgaG9wZSB0aGF0IGl0IHNob3VsZCBhY3R1YWxseSBjYXVzZSBjb21waWxlcgo+ ID4gd2FybmluZ3MgYWJvdXQgcG9zc2libHkgdW5pbml0aWFsaXplZCB2YXJpYWJsZXMgaWYgcGVv cGxlIHRoZW4gdXNlCj4gPiB0aGUKPiA+ICdwb3MnIHBvaW50ZXIgb3V0c2lkZSB0aGUgbG9vcC4g RXhjZXB0Cj4gPiAKPiA+ICAgKGEpIHRoYXQgY29kZSBpbiBzZ3gvZW5jbC5jIGN1cnJlbnRseSBp bml0aWFsaXplcyAndG1wJyB0byBOVUxMCj4gPiBmb3IKPiA+IGluZXhwbGljYWJsZSByZWFzb25z IC0gcG9zc2libHkgYmVjYXVzZSBpdCBhbHJlYWR5IGV4cGVjdGVkIHRoaXMKPiA+IGJlaGF2aW9y Cj4gPiAKPiA+ICAgKGIpIHdoZW4gSSByZW1vdmUgdGhhdCBOVUxMIGluaXRpYWxpemVyLCBJIHN0 aWxsIGRvbid0IGdldCBhCj4gPiB3YXJuaW5nLAo+ID4gYmVjYXVzZSB3ZSd2ZSBkaXNhYmxlZCAt V25vLW1heWJlLXVuaW5pdGlhbGl6ZWQgc2luY2UgaXQgcmVzdWx0cyBpbgo+ID4gc28KPiA+IG1h bnkgZmFsc2UgcG9zaXRpdmVzLgo+ID4gCj4gPiBPaCB3ZWxsLgo+ID4gCj4gPiBBbnl3YXksIGdp dmUgdGhpcyBwYXRjaCBhIGxvb2ssIGFuZCBhdCBsZWFzdCBpZiBpdCdzIGV4cGFuZGVkIHRvIGRv Cj4gPiAiKHBvcykgPSBOVUxMIiBpbiB0aGUgZW50cnkgc3RhdGVtZW50IGZvciB0aGUgZm9yLWxv b3AsIGl0IHdpbGwKPiA+IGF2b2lkIHRoZSBIRUFEIHR5cGUgY29uZnVzaW9uIHRoYXQgSmFrb2Ig aXMgd29ya2luZyBvbi4gQW5kIEkgdGhpbmsKPiA+IGluIGEgY2xlYW5lciB3YXkgdGhhbiB0aGUg aG9ycmlkIGdhbWVzIGhlIHBsYXlzLgo+ID4gCj4gPiAoQnV0IGl0IHdvbid0IGF2b2lkIHBvc3Np YmxlIENQVSBzcGVjdWxhdGlvbiBvZiBzdWNoIHR5cGUKPiA+IGNvbmZ1c2lvbi4gVGhhdCwgaW4g bXkgb3BpbmlvbiwgaXMgYSBjb21wbGV0ZWx5IGRpZmZlcmVudCBpc3N1ZSkKPiAKPiBZZXMsIGNv bXBsZXRlbHkgYWdyZWUuCj4gCj4gPiBJIGRvIHdpc2ggd2UgY291bGQgYWN0dWFsbHkgcG9pc29u IHRoZSAncG9zJyB2YWx1ZSBhZnRlciB0aGUgbG9vcAo+ID4gc29tZWhvdyAtIGJ1dCBjbGVhcmx5 IHRoZSAibWlnaHQgYmUgdW5pbml0aWFsaXplZCIgSSB3YXMgaG9waW5nIGZvcgo+ID4gaXNuJ3Qg dGhlIHdheSB0byBkbyBpdC4KPiA+IAo+ID4gQW55Ym9keSBoYXZlIGFueSBpZGVhcz8KPiAKPiBJ IHRoaW5rIHdlIHNob3VsZCBsb29rIGF0IHRoZSB1c2UgY2FzZXMgd2h5IGNvZGUgaXMgdG91Y2hp bmcgKHBvcykKPiBhZnRlciB0aGUgbG9vcC4KPiAKPiBKdXN0IGZyb20gc2tpbW1pbmcgb3ZlciB0 aGUgcGF0Y2hlcyB0byBjaGFuZ2UgdGhpcyBhbmQgZXhwZXJpZW5jZQo+IHdpdGggdGhlIGRyaXZl cnMvc3Vic3lzdGVtcyBJIGhlbHAgdG8gbWFpbnRhaW4gSSB0aGluayB0aGUgcHJpbWFyeQo+IHBh dHRlcm4gbG9va3Mgc29tZXRoaW5nIGxpa2UgdGhpczoKPiAKPiBsaXN0X2Zvcl9lYWNoX2VudHJ5 KGVudHJ5LCBoZWFkLCBtZW1iZXIpIHsKPiAgICAgIGlmIChzb21lX2NvbmRpdGlvbl9jaGVja2lu ZyhlbnRyeSkpCj4gICAgICAgICAgYnJlYWs7Cj4gfQo+IGRvX3NvbWV0aGluZ193aXRoKGVudHJ5 KTsKCgpBY3R1YWxseSwgd2UgdXN1YWxseSBoYXZlIGEgY2hlY2sgdG8gc2VlIGlmIHRoZSBsb29w IGZvdW5kIGFueXRoaW5nLApidXQgaW4gdGhhdCBjYXNlIGl0IHNob3VsZCBzb21ldGhpbmcgbGlr ZQoKaWYgKGxpc3RfZW50cnlfaXNfaGVhZChlbnRyeSwgaGVhZCwgbWVtYmVyKSkgewogICAgcmV0 dXJuIHdpdGggZXJyb3I7Cn0KZG9fc29tZXRoaW5fd2l0aChlbnRyeSk7CgpTdWZmaWNlPyAgVGhl IGxpc3RfZW50cnlfaXNfaGVhZCgpIG1hY3JvIGlzIGRlc2lnbmVkIHRvIGNvcGUgd2l0aCB0aGUK Ym9ndXMgZW50cnkgb24gaGVhZCBwcm9ibGVtLgoKSmFtZXMKCgoKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4LWYyZnMtZGV2ZWwgbWFpbGluZyBs aXN0CkxpbnV4LWYyZnMtZGV2ZWxAbGlzdHMuc291cmNlZm9yZ2UubmV0Cmh0dHBzOi8vbGlzdHMu c291cmNlZm9yZ2UubmV0L2xpc3RzL2xpc3RpbmZvL2xpbnV4LWYyZnMtZGV2ZWwK 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 64020C433FE for ; Tue, 1 Mar 2022 04:38:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D8C2810EB85; Tue, 1 Mar 2022 04:37:27 +0000 (UTC) X-Greylist: delayed 623 seconds by postgrey-1.36 at gabe; Mon, 28 Feb 2022 20:53:23 UTC Received: from bedivere.hansenpartnership.com (bedivere.hansenpartnership.com [IPv6:2607:fcd0:100:8a00::2]) by gabe.freedesktop.org (Postfix) with ESMTPS id F408F10E442; Mon, 28 Feb 2022 20:53:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from localhost (localhost [127.0.0.1]) by bedivere.hansenpartnership.com (Postfix) with ESMTP id 4748C1280EAE; Mon, 28 Feb 2022 15:42:59 -0500 (EST) Received: from bedivere.hansenpartnership.com ([127.0.0.1]) by localhost (bedivere.hansenpartnership.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i-njNjm3Pi4q; Mon, 28 Feb 2022 15:42:59 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=hansenpartnership.com; s=20151216; t=1646080979; bh=syYpb/xpUEM74XaWkn+xK6Z1vfQJZQo/NwVp9EVw5rs=; h=Message-ID:Subject:From:To:Date:In-Reply-To:References:From; b=ujxiIgy0T+0bXa5l/GTOdjcngh6b85JmDA0WyHri0Bbh2a93S9iJ0SxWyDfl/MyPo LfSkwtJLqYe+MRa0ZrGntCtSGoW1hjohSGvPcqjksKBYaUrg8Aw3UwyQBazEu7CyPQ sHqUpBIAmzvzOB+1XNyd+aN4TkXOB/egoLerum4w= Received: from jarvis.int.hansenpartnership.com (unknown [IPv6:2601:5c4:4300:c551::527]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bedivere.hansenpartnership.com (Postfix) with ESMTPSA id DDA6D12806A6; Mon, 28 Feb 2022 15:42:54 -0500 (EST) Message-ID: From: James Bottomley To: Christian =?ISO-8859-1?Q?K=F6nig?= , Linus Torvalds Date: Mon, 28 Feb 2022 15:42:53 -0500 In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Tue, 01 Mar 2022 04:37:19 +0000 Subject: Re: [Nouveau] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-wireless , alsa-devel@alsa-project.org, KVM list , "Gustavo A. R. Silva" , linux-iio@vger.kernel.org, nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel , Cristiano Giuffrida , "Bos, H.J." , linux1394-devel@lists.sourceforge.net, drbd-dev@lists.linbit.com, linux-arch , CIFS , linux-aspeed@lists.ozlabs.org, linux-scsi , linux-rdma , linux-staging@lists.linux.dev, amd-gfx list , Jason Gunthorpe , intel-wired-lan@lists.osuosl.org, kgdb-bugreport@lists.sourceforge.net, bcm-kernel-feedback-list@broadcom.com, Dan Carpenter , Linux Media Mailing List , Kees Cook , Arnd Bergman , Linux PM , intel-gfx , Brian Johannesmeyer , Nathan Chancellor , dma , Christophe JAILLET , Jakob Koschel , v9fs-developer@lists.sourceforge.net, linux-tegra , Thomas Gleixner , Andy Shevchenko , Linux ARM , linux-sgx@vger.kernel.org, linux-block , Netdev , linux-usb@vger.kernel.org, samba-technical@lists.samba.org, Linux Kernel Mailing List , Linux F2FS Dev Mailing List , tipc-discussion@lists.sourceforge.net, Linux Crypto Mailing List , linux-fsdevel , linux-mediatek@lists.infradead.org, Andrew Morton , linuxppc-dev , Mike Rapoport Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which > > requires > > that "-std=gnu11" that was discussed in the original thread). > > > > That will guarantee that the 'pos' parameter of > > list_for_each_entry() > > is only updated INSIDE the for_each_list_entry() loop, and can > > never > > point to the (wrongly typed) head entry. > > > > And I would actually hope that it should actually cause compiler > > warnings about possibly uninitialized variables if people then use > > the > > 'pos' pointer outside the loop. Except > > > > (a) that code in sgx/encl.c currently initializes 'tmp' to NULL > > for > > inexplicable reasons - possibly because it already expected this > > behavior > > > > (b) when I remove that NULL initializer, I still don't get a > > warning, > > because we've disabled -Wno-maybe-uninitialized since it results in > > so > > many false positives. > > > > Oh well. > > > > Anyway, give this patch a look, and at least if it's expanded to do > > "(pos) = NULL" in the entry statement for the for-loop, it will > > avoid the HEAD type confusion that Jakob is working on. And I think > > in a cleaner way than the horrid games he plays. > > > > (But it won't avoid possible CPU speculation of such type > > confusion. That, in my opinion, is a completely different issue) > > Yes, completely agree. > > > I do wish we could actually poison the 'pos' value after the loop > > somehow - but clearly the "might be uninitialized" I was hoping for > > isn't the way to do it. > > > > Anybody have any ideas? > > I think we should look at the use cases why code is touching (pos) > after the loop. > > Just from skimming over the patches to change this and experience > with the drivers/subsystems I help to maintain I think the primary > pattern looks something like this: > > list_for_each_entry(entry, head, member) { > if (some_condition_checking(entry)) > break; > } > do_something_with(entry); Actually, we usually have a check to see if the loop found anything, but in that case it should something like if (list_entry_is_head(entry, head, member)) { return with error; } do_somethin_with(entry); Suffice? The list_entry_is_head() macro is designed to cope with the bogus entry on head problem. James From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Date: Mon, 28 Feb 2022 15:42:53 -0500 Subject: [Intel-wired-lan] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr In-Reply-To: <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> References: <20220228110822.491923-1-jakobkoschel@gmail.com> <20220228110822.491923-3-jakobkoschel@gmail.com> <2e4e95d6-f6c9-a188-e1cd-b1eae465562a@amd.com> <282f0f8d-f491-26fc-6ae0-604b367a5a1a@amd.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On Mon, 2022-02-28 at 21:07 +0100, Christian K?nig wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian K?nig > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which > > requires > > that "-std=gnu11" that was discussed in the original thread). > > > > That will guarantee that the 'pos' parameter of > > list_for_each_entry() > > is only updated INSIDE the for_each_list_entry() loop, and can > > never > > point to the (wrongly typed) head entry. > > > > And I would actually hope that it should actually cause compiler > > warnings about possibly uninitialized variables if people then use > > the > > 'pos' pointer outside the loop. Except > > > > (a) that code in sgx/encl.c currently initializes 'tmp' to NULL > > for > > inexplicable reasons - possibly because it already expected this > > behavior > > > > (b) when I remove that NULL initializer, I still don't get a > > warning, > > because we've disabled -Wno-maybe-uninitialized since it results in > > so > > many false positives. > > > > Oh well. > > > > Anyway, give this patch a look, and at least if it's expanded to do > > "(pos) = NULL" in the entry statement for the for-loop, it will > > avoid the HEAD type confusion that Jakob is working on. And I think > > in a cleaner way than the horrid games he plays. > > > > (But it won't avoid possible CPU speculation of such type > > confusion. That, in my opinion, is a completely different issue) > > Yes, completely agree. > > > I do wish we could actually poison the 'pos' value after the loop > > somehow - but clearly the "might be uninitialized" I was hoping for > > isn't the way to do it. > > > > Anybody have any ideas? > > I think we should look at the use cases why code is touching (pos) > after the loop. > > Just from skimming over the patches to change this and experience > with the drivers/subsystems I help to maintain I think the primary > pattern looks something like this: > > list_for_each_entry(entry, head, member) { > if (some_condition_checking(entry)) > break; > } > do_something_with(entry); Actually, we usually have a check to see if the loop found anything, but in that case it should something like if (list_entry_is_head(entry, head, member)) { return with error; } do_somethin_with(entry); Suffice? The list_entry_is_head() macro is designed to cope with the bogus entry on head problem. James