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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130607163658.GC25323@logfs.org>
Date:	Fri, 7 Jun 2013 12:36:58 -0400
From:	Jörn Engel <joern@...fs.org>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Chris Mason <chris.mason@...ionio.com>,
	linux-btrfs@...r.kernel.org
Subject: Re: [PATCH 1/2] list: add list_for_each_entry_del

On Thu, 6 June 2013 22:49:22 +0300, Andy Shevchenko wrote:
> On Thu, Jun 6, 2013 at 9:12 PM, Jörn Engel <joern@...fs.org> wrote:
> > On Thu, 6 June 2013 22:32:55 +0300, Andy Shevchenko wrote:
> >> On Mon, Jun 3, 2013 at 8:28 PM, Joern Engel <joern@...fs.org> wrote:
> >> > I have seen a lot of boilerplate code that either follows the pattern of
> >> >         while (!list_empty(head)) {
> >> >                 pos = list_entry(head->next, struct foo, list);
> >> >                 list_del(pos->list);
> >> >                 ...
> >> >         }
> >> > or some variant thereof.
> >>
> >> What the problem to use list_for_each_safe()?
> >
> > The loop may terminate with elements left on the list.  There is more,
> > but I would consider this the main problem.
> 
> I didn't quite get what you mean.

Take two threads, one doing a list_for_each_entry_safe loop and
dropping the lock after list_del, the other doing list_add.  Result is
that you finish the list_for_each_entry_safe loop with something
remaining on the list.

spin_lock
list_for_each_entry_safe
	list_del
	spin_unlock
			spin_lock
			list_add
			spin_unlock

If you search for this pattern in the kernel, you won't find too many
examples.  Quite likely that is because a) people realized this and
used a while (!list_empty()) loop to begin with or b) they started out
wrong and fixed the bug later.  Not sure how many examples of b) there
are.

At any rate, this is a purely janitorial patch.  It is almost by
definition of moderate utility and if there is significant opposition
or the patch actually causes harm in some way, we should drop it.

Jörn

--
Time? What's that? Time is only worth what you do with it.
-- Theo de Raadt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ