[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 3 Mar 2022 04:58:23 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Xiaomeng Tong' <xiam0nd.tong@...il.com>
CC: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"arnd@...db.de" <arnd@...db.de>,
"bcm-kernel-feedback-list@...adcom.com"
<bcm-kernel-feedback-list@...adcom.com>,
"bjohannesmeyer@...il.com" <bjohannesmeyer@...il.com>,
"c.giuffrida@...nl" <c.giuffrida@...nl>,
"christian.koenig@....com" <christian.koenig@....com>,
"christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>,
"dan.carpenter@...cle.com" <dan.carpenter@...cle.com>,
"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"drbd-dev@...ts.linbit.com" <drbd-dev@...ts.linbit.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"gustavo@...eddedor.com" <gustavo@...eddedor.com>,
"h.j.bos@...nl" <h.j.bos@...nl>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"jakobkoschel@...il.com" <jakobkoschel@...il.com>,
"jgg@...pe.ca" <jgg@...pe.ca>,
"keescook@...omium.org" <keescook@...omium.org>,
"kgdb-bugreport@...ts.sourceforge.net"
<kgdb-bugreport@...ts.sourceforge.net>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
"linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
"linux-cifs@...r.kernel.org" <linux-cifs@...r.kernel.org>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-f2fs-devel@...ts.sourceforge.net"
<linux-f2fs-devel@...ts.sourceforge.net>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
"linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"linux1394-devel@...ts.sourceforge.net"
<linux1394-devel@...ts.sourceforge.net>,
"linux@...musvillemoes.dk" <linux@...musvillemoes.dk>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"nathan@...nel.org" <nathan@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
"rppt@...nel.org" <rppt@...nel.org>,
"samba-technical@...ts.samba.org" <samba-technical@...ts.samba.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"tipc-discussion@...ts.sourceforge.net"
<tipc-discussion@...ts.sourceforge.net>,
"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
"v9fs-developer@...ts.sourceforge.net"
<v9fs-developer@...ts.sourceforge.net>
Subject: RE: [PATCH 2/6] treewide: remove using list iterator after loop body
as a ptr
From: Xiaomeng Tong
> Sent: 03 March 2022 02:27
>
> On Wed, 2 Mar 2022 14:04:06 +0000, David Laight
> <David.Laight@...LAB.COM> wrote:
> > I think that it would be better to make any alternate loop macro
> > just set the variable to NULL on the loop exit.
> > That is easier to code for and the compiler might be persuaded to
> > not redo the test.
>
> No, that would lead to a NULL dereference.
Why, it would make it b ethe same as the 'easy to use':
for (item = head; item; item = item->next) {
...
if (...)
break;
...
}
if (!item)
return;
> The problem is the mis-use of iterator outside the loop on exit, and
> the iterator will be the HEAD's container_of pointer which pointers
> to a type-confused struct. Sidenote: The *mis-use* here refers to
> mistakely access to other members of the struct, instead of the
> list_head member which acutally is the valid HEAD.
The problem is that the HEAD's container_of pointer should never
be calculated at all.
This is what is fundamentally broken about the current definition.
> IOW, you would dereference a (NULL + offset_of_member) address here.
Where?
> Please remind me if i missed something, thanks.
>
> Can you share your "alternative definitions" details? thanks!
The loop should probably use as extra variable that points
to the 'list node' in the next structure.
Something like:
for (xxx *iter = head->next;
iter == &head ? ((item = NULL),0) : ((item = list_item(iter),1));
iter = item->member->next) {
...
With a bit of casting you can use 'item' to hold 'iter'.
>
> > OTOH there may be alternative definitions that can be used to get
> > the compiler (or other compiler-like tools) to detect broken code.
> > Even if the definition can't possibly generate a working kerrnel.
>
> The "list_for_each_entry_inside(pos, type, head, member)" way makes
> the iterator invisiable outside the loop, and would be catched by
> compiler if use-after-loop things happened.
It is also a compete PITA for anything doing a search.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists