| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d245f691cfdf43d9a1e1e33acb570325@AcuMS.aculab.com>
Date: Tue, 1 Mar 2022 02:15:09 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...ux-foundation.org>,
Christian König <christian.koenig@....com>
CC: linux-wireless <linux-wireless@...r.kernel.org>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
KVM list <kvm@...r.kernel.org>,
"Gustavo A. R. Silva" <gustavo@...eddedor.com>,
"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
"Rasmus Villemoes" <linux@...musvillemoes.dk>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Cristiano Giuffrida <c.giuffrida@...nl>,
"Bos, H.J." <h.j.bos@...nl>,
"linux1394-devel@...ts.sourceforge.net"
<linux1394-devel@...ts.sourceforge.net>,
"drbd-dev@...ts.linbit.com" <drbd-dev@...ts.linbit.com>,
linux-arch <linux-arch@...r.kernel.org>,
CIFS <linux-cifs@...r.kernel.org>,
"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
linux-scsi <linux-scsi@...r.kernel.org>,
linux-rdma <linux-rdma@...r.kernel.org>,
"linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
amd-gfx list <amd-gfx@...ts.freedesktop.org>,
Jason Gunthorpe <jgg@...pe.ca>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"kgdb-bugreport@...ts.sourceforge.net"
<kgdb-bugreport@...ts.sourceforge.net>,
"bcm-kernel-feedback-list@...adcom.com"
<bcm-kernel-feedback-list@...adcom.com>,
Dan Carpenter <dan.carpenter@...cle.com>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
Kees Cook <keescook@...omium.org>,
"Arnd Bergman" <arnd@...db.de>,
Linux PM <linux-pm@...r.kernel.org>,
intel-gfx <intel-gfx@...ts.freedesktop.org>,
Brian Johannesmeyer <bjohannesmeyer@...il.com>,
Nathan Chancellor <nathan@...nel.org>,
dma <dmaengine@...r.kernel.org>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
Jakob Koschel <jakobkoschel@...il.com>,
"v9fs-developer@...ts.sourceforge.net"
<v9fs-developer@...ts.sourceforge.net>,
linux-tegra <linux-tegra@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
"linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
linux-block <linux-block@...r.kernel.org>,
Netdev <netdev@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"samba-technical@...ts.samba.org" <samba-technical@...ts.samba.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux F2FS Dev Mailing List
<linux-f2fs-devel@...ts.sourceforge.net>,
"tipc-discussion@...ts.sourceforge.net"
<tipc-discussion@...ts.sourceforge.net>,
Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
Andrew Morton <akpm@...ux-foundation.org>,
linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
Mike Rapoport <rppt@...nel.org>
Subject: RE: [PATCH 2/6] treewide: remove using list iterator after loop body
as a ptr
From: Linus Torvalds
> Sent: 28 February 2022 19:56
> On Mon, Feb 28, 2022 at 4:19 AM Christian König
> <christian.koenig@....com> 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)
>
> 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?
>
> Linus
diff --git a/include/linux/list.h b/include/linux/list.h
index dd6c2041d09c..bab995596aaa 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -634,10 +634,10 @@ static inline void list_splice_tail_init(struct list_head *list,
* @head: the head for your list.
* @member: the name of the list_head within the struct.
*/
-#define list_for_each_entry(pos, head, member) \
- for (pos = list_first_entry(head, typeof(*pos), member); \
- !list_entry_is_head(pos, head, member); \
- pos = list_next_entry(pos, member))
+#define list_for_each_entry(pos, head, member) \
+ for (typeof(pos) __iter = list_first_entry(head, typeof(*pos), member); \
+ !list_entry_is_head(__iter, head, member) && (((pos)=__iter),1); \
+ __iter = list_next_entry(__iter, member))
/**
* list_for_each_entry_reverse - iterate backwards over list of given type.
I think you actually want:
!list_entry_is_head(__iter, head, member) ? (((pos)=__iter),1) : (((pos) = NULL),0);
Which can be done in the original by:
!list_entry_is_head(pos, head, member) ? 1 : (((pos) = NULL), 0);
Although it would be safer to have (without looking up the actual name):
for (item *__item = head; \
__item ? (((pos) = list_item(__item, member)), 1) : (((pos) = NULL), 0);
__item = (pos)->member)
The local does need 'fixing' to avoid shadowing for nested loops.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists