linux-kernel - Re: [PATCH] futex: fix a race condition between REQUEUE

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]

Date:	Sat, 25 Oct 2014 20:19:48 -0400
From:	Brian Silverman <bsilver16384@...il.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Austin Schuh <austin.linux@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Darren Hart <darren@...art.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] futex: fix a race condition between REQUEUE_PI and task death

On Sat, 25 Oct 2014, Thomas Gleixner wrote:

> > pi_state_free and exit_pi_state_list both clean up futex_pi_state's.
> > exit_pi_state_list takes the hb lock first, and most callers of
> > pi_state_free do too. requeue_pi didn't, which causes lots of problems.
>
> "causes lots of problems" is not really a good explanation of the root
> cause. That wants a proper description of the race, i.e.
>
> CPU 0              CPU 1
> ...                ....
>
> I'm surely someone who is familiar with that code, but it took me
> quite some time to understand whats going on. The casual reader will
> just go into brain spiral mode and give up.

Thinking about it again, I'm no longer so sure that exit_pi_state_list is the
only place that it can race against. However, I did catch that one with a
particularly lucky crashdump, so I know it _can_ happen there. Is just
giving an example for that good?

> >  static void free_pi_state(struct futex_pi_state *pi_state)
>
> > @@ -1558,6 +1552,14 @@ retry_private:
> >               ret = get_futex_value_locked(&curval, uaddr1);
> >
> >               if (unlikely(ret)) {
> > +                     if (flags & FLAGS_SHARED && pi_state != NULL) {
>
> Why is this dependend on "flags & FLAGS_SHARED"? The shared/private
> property has nothing to do with that at all, but I might be missing
> something.

Nothing... Good catch. It was a bad rebase.

Thanks,

Brian
--
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/