From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 68F887A for ; Wed, 2 Mar 2022 00:04:15 +0000 (UTC) Received: by mail-ed1-f54.google.com with SMTP id h15so39250edv.7 for ; Tue, 01 Mar 2022 16:04:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=7DBSI5LsuTX3zYSZtOIoy3oAsGDBcqEZPI6D2l8hLXefxBFWI6DiumnIL8+/kq8ujm IxUnI83+Wb1EReJrDOJU4jIfUbvIwGd5u/QlJI8V5O8jIF8UHD7WFFTZLWB7GO2C6OZP RfWJ7ZlWaFdiv72Z0HGp7paM1qDWDfaSIS8Zs1nDe3CSlAKGeC4VnBjGHiiduE2UxTRx kDqSouUXWp/JaVMlCk69+IGbWkbzTPW00duKzp+BBiZ7nEzXXo/KcCAMq6aIOBRrmxwk COatIcNiOJAk54Ue8KePa2rJNFvjBy3kKvm9986PrmDcKKFp++OM2O8wlL/e8w9HzCEq J78g== X-Gm-Message-State: AOAM531FlIfSEH92emG1OyPkAOTaHCq9axdh9axtPIvYG75ivSyPT5Kp w/PV5g8ceuve1IIGJ6iMIS1OW5wdjoOqeMVqA0A= X-Google-Smtp-Source: ABdhPJzdrFprct9bddrJxoyTJDot5KXLb83/9AEkNkm9i5OwURfNgQyGqCWGVgahuduWjGE20Gz1Rg== X-Received: by 2002:aa7:c896:0:b0:413:c96d:6bfd with SMTP id p22-20020aa7c896000000b00413c96d6bfdmr9876285eds.331.1646179453437; Tue, 01 Mar 2022 16:04:13 -0800 (PST) Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com. [209.85.218.46]) by smtp.gmail.com with ESMTPSA id t4-20020a056402524400b00415b90801edsm130027edd.57.2022.03.01.16.04.13 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 16:04:13 -0800 (PST) Received: by mail-ej1-f46.google.com with SMTP id gb39so294529ejc.1 for ; Tue, 01 Mar 2022 16:04:13 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr To: David Laight Cc: James Bottomley , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport Content-Type: text/plain; charset="UTF-8" On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. 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 4C203C433F5 for ; Wed, 2 Mar 2022 09:01:34 +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 6F4171FA6; Wed, 2 Mar 2022 10:00:42 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 6F4171FA6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1646211692; bh=3kbm6Zn83WYHm4p7NlsBp8rCF7BbMMW2MixJxWkUISU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=mq+kqk8T3mLy2OWbbwflqFE8/avzeWG35kyK1iDP5azKWrQ6Nl82DJiPIuK6fq+rm JZKa7M8HfunpKGFCwkWgBHKniFp19dpmogY7iWStZ+mR7cgDOW2oNN2hkezloDp5Jc KoKnrmtI1WsCOrt0xtByCxTsP5yW+RoLvPNA4E3Y= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 5E1CCF8087C; Wed, 2 Mar 2022 09:34:36 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id A01E3F80227; Wed, 2 Mar 2022 00:56:08 +0100 (CET) Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 91F66F80167 for ; Wed, 2 Mar 2022 00:56:02 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 91F66F80167 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="A1xSV6w+" Received: by mail-lf1-x131.google.com with SMTP id y24so29624717lfg.1 for ; Tue, 01 Mar 2022 15:56:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=xrd6Yf21qCK2wszoJFDFoguv63TZLjaM+bYw3bKC1UqOGukfpQZKJrI/65zU5WgmKR XUGEMkv76SQmkAaLOgc2oU4YHa/jLhA9R2ezfKieSknT4MuozBuwqn6KpbLervzUzCvf Mu6qOY5JxxCT5gSwTjEw7hmJHdB12/bMdZwFXP87iP9lewIdb1Gl6BENNirRgb5wi+9E h4Ww9Vl9wBqkNQY/Bcc8vKbIJTrzjLXuh9/pjs1niIuJIF2KlJ/6bxMrZh0mIFuZdWa7 nzf0vvTxpJmC7Uqd0rUui6fG0mqIwmrd2FRckFk58EvkeNi1tFK+3gy8VmbRb9/i/6Oe c2ew== X-Gm-Message-State: AOAM533iltsF5Uu0ng5X9tk0sColhp5QMCKApGxzIwBrnherNlr3yfPJ 5okr0KYF3Cgk/gnfalmLJz9mXtbSwoOQD2Q4rHE= X-Google-Smtp-Source: ABdhPJwEhD71OFJomrMrx+Ui2DzWY/YHEMRT5Z8Ob/XF5AXxJCgOXC2vHU71xVOEWMACA19i+M1qwQ== X-Received: by 2002:a05:6512:16a3:b0:445:8f73:24b0 with SMTP id bu35-20020a05651216a300b004458f7324b0mr10781943lfb.181.1646178960331; Tue, 01 Mar 2022 15:56:00 -0800 (PST) Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com. [209.85.167.44]) by smtp.gmail.com with ESMTPSA id g36-20020a0565123ba400b004437f7cae9dsm1720413lfv.219.2022.03.01.15.55.58 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 15:55:59 -0800 (PST) Received: by mail-lf1-f44.google.com with SMTP id y24so29624616lfg.1 for ; Tue, 01 Mar 2022 15:55:58 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr To: David Laight Content-Type: text/plain; charset="UTF-8" X-Mailman-Approved-At: Wed, 02 Mar 2022 09:33:36 +0100 Cc: "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 , James Bottomley , Cristiano Giuffrida , "Bos, H.J." , "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" , 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 , linux-fsdevel , 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" , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , 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 Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. 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 872BDC433F5 for ; Wed, 2 Mar 2022 00:02:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8A0AB10E8D2; Wed, 2 Mar 2022 00:02:09 +0000 (UTC) Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by gabe.freedesktop.org (Postfix) with ESMTPS id D616E10E8D2 for ; Wed, 2 Mar 2022 00:02:07 +0000 (UTC) Received: by mail-ed1-x533.google.com with SMTP id s24so42701edr.5 for ; Tue, 01 Mar 2022 16:02:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=U6pxfwpWh8zX98k8KoPyCgenPpoCTBYZsvU+0D29Rec50g8UAA+nO95nMBv9uF1b4G RBKhQotWEKY4vnD8SeL0LMAXGjYeIEfNsygUQJQ6nF0T5PJHpNwpOr3jwYov2JIHXP/U 5bWuR5puY+QDywrOKpt2A3IQULI8goi9myD6ufjljpEQyFOacoJm0x7RFo74eWp1MCDB mKaUeCA2JadKeSX/FWU/RaALBffC+DPYq6en6thgNqeSTBAAS87ZDt8noG3fMoWU/uHM NiGA0NTKy4JcxPxiHsMmkTvt1AUcAguse4BDtcdAHzmF4cnUa93WxqBYvhqu7WVdhvz0 eKFw== X-Gm-Message-State: AOAM532+Qa8EJBSg4Aa94+j8tId0d0n0IjZljo4HBkxsy9fsybjK1puT ybM6okCPldcEzV/bWKMvyz50z+vAxjUhW5AQORE= X-Google-Smtp-Source: ABdhPJyegefZRmg0u3A1ASlQK1HB0YvicbBZI/PrUB/7w+8LSVMWmodoYajmVC2cAQsImVTKN8o8lw== X-Received: by 2002:a50:d707:0:b0:415:a24c:2a6d with SMTP id t7-20020a50d707000000b00415a24c2a6dmr2219341edi.140.1646179326097; Tue, 01 Mar 2022 16:02:06 -0800 (PST) Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com. [209.85.221.48]) by smtp.gmail.com with ESMTPSA id b16-20020aa7d490000000b00415a0958366sm725200edr.88.2022.03.01.16.02.05 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 16:02:05 -0800 (PST) Received: by mail-wr1-f48.google.com with SMTP id bk29so150765wrb.4 for ; Tue, 01 Mar 2022 16:02:05 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: To: David Laight Content-Type: text/plain; charset="UTF-8" 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: "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 , James Bottomley , Cristiano Giuffrida , "Bos, H.J." , "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" , 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 , linux-fsdevel , 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" , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. 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 5CCBDC433EF for ; Wed, 2 Mar 2022 00:03:27 +0000 (UTC) Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.94.2) (envelope-from ) id 1nPCSk-00077W-Hk; Wed, 02 Mar 2022 00:03:25 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nPCSd-00077L-Mq for linux-f2fs-devel@lists.sourceforge.net; Wed, 02 Mar 2022 00:03:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Sender:Reply-To:Content-Transfer-Encoding :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=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=IYooCL2bErS8C+LVK8D5E90GiP rwzBWln/tHuGF0ZEOOB8gakNcVN+nNCdDS8I9T3ocyo8H4oX0Qb+38mReX0L2E1UQq/+HrL2yxuYi 0KgYo6N1eh0dupzs0RyVjC/M+/n4B1mcwnz8x8mKC6osZC+OqWS48mdHXcZgzof7e3tU=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Cc:To:Subject:Message-ID:Date:From:In-Reply-To:References: MIME-Version:Sender:Reply-To:Content-Transfer-Encoding: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=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=S0iSlD24auOsZdSR8vOHzdOrRY 8XNM/zmbvot34AMyMfN8sH3QlJbE5O36zwJyGlgXmqKWLlTO8OuJIAK6gt9vU7OACrxYJ+vUx0UN9 oSWedhCWxp9vP0FXA42C2bHmHPzTXAT61yCnu5LIKANoIkPy3Twv+3Tyoi1rUAjzHGpQ=; Received: from mail-lf1-f41.google.com ([209.85.167.41]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.94.2) id 1nPCSY-0007Om-3x for linux-f2fs-devel@lists.sourceforge.net; Wed, 02 Mar 2022 00:03:17 +0000 Received: by mail-lf1-f41.google.com with SMTP id y24so29648181lfg.1 for ; Tue, 01 Mar 2022 16:03:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=3ZkbuBBXTeAsrpKM41DwEk+fZmjC+5ngVYN9BT9UTJErcMsEkCZH6lPIhhI89jSKMO ZLcygmpHDfOAsyaO2wqycDQP1DVAsbKMLFLZpNqNZsLATRSkP+iBLOcNmk41WwYLc3R5 Lv1XJTPVhoWUtw5886hB0ncmEbhVCcKAMj9mEh2qBKXNt2Pf02zD8geijzWMcqymatyu E0FYr3t0lODuPK5Jhn7AQFg0qyjHakTKmE9dm6iAmL9mLdu+KIIsHMaW8MuaWWek8A5F u7NQnFupiqLWSdJGd5+1AjDdjvI7yvmRCd+LeO2+SYtOBXILDTE1fhP+GfmUgW2PhV5A 5eGg== X-Gm-Message-State: AOAM5333AodvC/5r7dKQC+9vyS1C1ojc3o2wdkUwCbWvzt/wYnCWLWzu coAOtjwEVY3AK+NnlV8hL+uR+RZfmip2Ibn2Sw8= X-Google-Smtp-Source: ABdhPJwAyoStVqKMVW8Cvrd0BZXBm9eF7rfn+1YYb8+6ceU9Tj0cxVC2e3nr5kcC7AEIJCFRLV0gaw== X-Received: by 2002:a19:6a16:0:b0:442:2062:f7a with SMTP id u22-20020a196a16000000b0044220620f7amr16531139lfu.456.1646179387239; Tue, 01 Mar 2022 16:03:07 -0800 (PST) Received: from mail-lj1-f179.google.com (mail-lj1-f179.google.com. [209.85.208.179]) by smtp.gmail.com with ESMTPSA id f6-20020a2eb386000000b0024624f70b13sm2253527lje.136.2022.03.01.16.03.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 16:03:07 -0800 (PST) Received: by mail-lj1-f179.google.com with SMTP id e8so50073ljj.2 for ; Tue, 01 Mar 2022 16:03:06 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: To: David Laight X-Headers-End: 1nPCSY-0007Om-3x 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: "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 , James Bottomley , Cristiano Giuffrida , "Bos, H.J." , "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" , 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 , linux-fsdevel , 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" , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel 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 E5050C433EF for ; Wed, 2 Mar 2022 00:04:22 +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:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=SwDaF3I9yB1+3XS5HJhWN/kHnIzjsgg/rJMWvS5JocA=; b=HgLJD8efn9tOKn U5kXZ03OPjVktVtn9VMYPy93O9o/L8AlKMWd++L9KwkGBW0W54WjDF8nk+oCJCsgUUSuUJLH0kDAr 8+OdP6fklN6gyANbZ2GZTaKtTepR2TuLqs7SLUwk4lVdbZsnda+/WT3+fcm8ovKZfax+sUqqn/PSx zjhw/6roWl33O+Xh9pz5hBVYEJpARtiIYSIdv5RGq1uhgsqmBZjsZn+Jff/hMRFp3mCmsXB5cLkc/ MSxSsI+xN3glUmb+/9/Tv4uozkyi590avPLdFYD7VoIOV9it82T7g+K3U+ij/YQkMcYlweJSw7y7r V2On/Po1mA7RD72XJ25A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nPCTU-000tce-IQ; Wed, 02 Mar 2022 00:04:12 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nPCTR-000tcL-8P for linux-mediatek@lists.infradead.org; Wed, 02 Mar 2022 00:04:10 +0000 Received: by mail-lf1-x12b.google.com with SMTP id u20so29653261lff.2 for ; Tue, 01 Mar 2022 16:04:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=UqdiQKA3shjCZlJXmrp+car2BiogDjvXvra/RBng7rPi6tDLg2WbdSYi/DivM5e+bS X+nGEonxXWfoGWkoDveUiS1ZlK57B35Yob6YM8AKdV55TB4GpSVrWIMxhisLS+/sk5Y1 fS0GX0BCM4oq1TLgwU5pcn2wmHicQ5MRgZh/4N9YXPjrs1C/9eK5QTovdg006YgJF59K DOcLpRX11QDyh1kLQgU2va3GIyx+7pB5lEapsM0JotD7hDLMjLjv1OM5Npb3/+xH8I3e Ks2HHO8WGV6BLt2oSxk8Z2RWEmhd5P6p9C8U8Y+uawi4e2spihW3xfW+oNhh5dxSZnje Bt3A== X-Gm-Message-State: AOAM530/iQMssIBBlEwGV7Ct5osZQElpTd2ux49FuyfyFD2R4DOHhmf4 pOvzSV62WxUbU95PlRuzy3duc16ID/1tUHExIBk= X-Google-Smtp-Source: ABdhPJyKuYwKPtR4f8l5iAqFcNvzTbSSWzd3P78fDGBPRqePtnRAFXXqnQT9CCzz/9p8E32q5aUYDg== X-Received: by 2002:ac2:51cd:0:b0:443:675b:b1bc with SMTP id u13-20020ac251cd000000b00443675bb1bcmr16732414lfm.164.1646179447381; Tue, 01 Mar 2022 16:04:07 -0800 (PST) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com. [209.85.167.52]) by smtp.gmail.com with ESMTPSA id b17-20020a2e9891000000b00247a36b1360sm463461ljj.28.2022.03.01.16.04.07 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 16:04:07 -0800 (PST) Received: by mail-lf1-f52.google.com with SMTP id d23so29564833lfv.13 for ; Tue, 01 Mar 2022 16:04:07 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr To: David Laight Cc: James Bottomley , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220301_160409_311835_32B184BC X-CRM114-Status: GOOD ( 19.95 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 EA282C433F5 for ; Wed, 2 Mar 2022 04:57:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 55CC210EB03; Wed, 2 Mar 2022 04:57:31 +0000 (UTC) Received: from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com [IPv6:2a00:1450:4864:20::22f]) by gabe.freedesktop.org (Postfix) with ESMTPS id 168F910E8D2 for ; Wed, 2 Mar 2022 00:04:12 +0000 (UTC) Received: by mail-lj1-x22f.google.com with SMTP id l12so12181177ljh.12 for ; Tue, 01 Mar 2022 16:04:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=A1xSV6w+mnGI8KddmZFT+SVp47UhRqZlj7dZE7Ue1BgWjGw2+QCc51Zb6PIPvKiXuh /TGd1P3o63R0kiFyLwVU+kzDBf6Kt9wFSZwwJYnrWAqavu9szp3nfmKm0Qe68N3l8Mdl bWvesgtwBf4BWRSm4aUiCmj0pmAaHYwmzAjUs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4H0hy1V+vsSL/J4iRHU98fs6Ghx9E/bawvPg5OjXQmQ=; b=bkYjt6cR+dUhtL5KrNzGlomenBT5HSOnpg6noGJZ8tdRWkEphT3bPCcSeAiKC5tYqm LS9YjCV77dbxPfUIKyQOoYWGkQT7pe1ZYnc7x3RKg6gkn2MZ1WNCNV2aHBmkgA9lhJF4 Y6bjSTYDBIXTVHjND18yQDpVwHYH9J7zPeNAOd96XkfCL5t23/EutO758AW4Wu+tMpQX Bd0XhsrCCWfu2dznD4u1wjhZ1xwZhjtvNlx2wV4DU/taK4SsxaTg9/QqnoZDxz6GS4Mn hGvsyD+sNdoy1zPQ7lyV9PzVwb/P1L+tXYSU54MLT4SP+dxBvNj4IGkZvx/Xb2wZQxQb iUmQ== X-Gm-Message-State: AOAM530MLMppVB4ODEpPvUPZyAHWR2eQkflUdjGC8rV3b5vVQYRaew7u apZ48UGD8JZrmJemUbK3GloV/hwUSsJyOBDzXfs= X-Google-Smtp-Source: ABdhPJz4RbT6SC7wDdHE6CJdInLSu5iydCKL99Y9cm2Lkzbce/McRa9xxl+5paa7FEbrJojuaSxRBw== X-Received: by 2002:a2e:a60c:0:b0:246:4739:4b40 with SMTP id v12-20020a2ea60c000000b0024647394b40mr18812938ljp.526.1646179450139; Tue, 01 Mar 2022 16:04:10 -0800 (PST) Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com. [209.85.208.181]) by smtp.gmail.com with ESMTPSA id r6-20020a19ac46000000b004433cbb7ef9sm1725902lfc.137.2022.03.01.16.04.09 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 16:04:09 -0800 (PST) Received: by mail-lj1-f181.google.com with SMTP id l12so12181130ljh.12 for ; Tue, 01 Mar 2022 16:04:09 -0800 (PST) X-Received: by 2002:a05:6512:3042:b0:437:96f5:e68a with SMTP id b2-20020a056512304200b0043796f5e68amr17643498lfb.449.1646178958685; Tue, 01 Mar 2022 15:55:58 -0800 (PST) MIME-Version: 1.0 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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.com> From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 X-Gmail-Original-Message-ID: Message-ID: To: David Laight Content-Type: text/plain; charset="UTF-8" X-Mailman-Approved-At: Wed, 02 Mar 2022 04:57:26 +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: "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 , James Bottomley , Cristiano Giuffrida , "Bos, H.J." , "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" , 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 , linux-fsdevel , 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" , 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 , =?UTF-8?Q?Christian_K=C3=B6nig?= , Mike Rapoport Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Date: Tue, 1 Mar 2022 15:55:42 -0800 Subject: [Intel-wired-lan] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr In-Reply-To: <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.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> <7D0C2A5D-500E-4F38-AD0C-A76E132A390E@kernel.org> <73fa82a20910c06784be2352a655acc59e9942ea.camel@HansenPartnership.com> <7dc860874d434d2288f36730d8ea3312@AcuMS.aculab.com> <0ced2b155b984882b39e895f0211037c@AcuMS.aculab.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 Tue, Mar 1, 2022 at 3:19 PM David Laight wrote: > > Having said that there are so few users of list_entry_is_head() > it is reasonable to generate two new names. Well, the problem is that the users of list_entry_is_head() may be few - but there are a number of _other_ ways to check "was that the HEAD pointer", and not all of them are necessarily correct. IOW, different places do different random tests for "did we walk the whole loop without breaking out". And many of them happen to work. In fact, in practice, pretty much *all* of them happen to work, and you have to have the right struct layout and really really bad luck to hit a case of "type confusion ended up causing the test to not work". And *THAT* is the problem here. It's not the "there are 25ish places that current use list_entry_is_head()". It's the "there are ~480 places that use the type-confused HEAD entry that has been cast to the wrong type". And THAT is why I think we'd be better off with that bigger change that simply means that you can't use the iterator variable at all outside the loop, and try to make it something where the compiler can help catch mis-uses. Now, making the list_for_each_entry() thing force the iterator to NULL at the end of the loop does fix the problem. The issue I have with it is really just that you end up getting no warning at all from the compiler if you mix old-style and new-style semantics. Now, you *will* get an oops (if using a new-style iterator with an old-style check), but many of these things will be in odd driver code and may happen only for error cases. And if you use a new-style check with an old-style iterator (ie some backport problem), you will probably end up getting random memory corruption, because you'll decide "it's not a HEAD entry", and then you'll actually *use* the HEAD that has the wrong type cast associated with it. See what my worry is? With the "don't use iterator outside the loop" approach, the exact same code works in both the old world order and the new world order, and you don't have the semantic confusion. And *if* you try to use the iterator outside the loop, you'll _mostly_ (*) get a compiler warning about it not being initialized. Linus (*) Unless somebody initializes the iterator pointer pointlessly. Which clearly does happen. Thus the "mostly". It's not perfect, and that's most definitely not nice - but it should at least hopefully make it that much harder to mess up.