linux-kernel - Re: [PATCH 1/2] Update woken requeued futex_q lock

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:	Thu, 06 Aug 2009 17:24:41 -0700
From:	Darren Hart <dvhltc@...ibm.com>
To:	linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org
CC:	tglx@...utronix.de, peterz@...radead.org, rostedt@...dmis.org,
	mingo@...e.hu, dino@...ibm.com, johnstul@...ibm.com,
	John Kacur <jkacur@...hat.com>
Subject: Re: [PATCH 1/2] Update woken requeued futex_q lock_ptr

Darren Hart wrote:
> Darren Hart wrote:
>> futex_requeue() can acquire the lock on behalf of a waiter during the 
>> requeue
>> loop in the event of a lock steal or owner died. 
>> futex_wait_requeue_pi() cleans
>> up the pi_state owner, using the lock_ptr to protect against 
>> concurrent access
>> to the pi_state.  The pi_state is found on the requeue target futex 
>> hash bucket
>> so the lock_ptr needs to be updated accordingly.  The problem 
>> manifested by
>> triggering the WARN_ON in lookup_pi_state() about the pid != 
>> pi_state->owner
>> pid.
>> The astute reviewer will note that still exists a race between the time
>> futex_requeue() releases hb2->lock() and the time when 
>> futex_wait_requeue_pi()
>> acquires it.  During this time the pi_state and the futex uaddr are 
>> not in sync
>> with the rt_mutex ownership.  This patch closes the window to the 
>> point where
>> my tests now pass, but we still need to address it.
>>
>> Note: Please apply to mainline and rt
>>
> 
> 
>> static inline
>> -void requeue_pi_wake_futex(struct futex_q *q, union futex_key *key)
>> +void requeue_pi_wake_futex(struct futex_q *q, union futex_key *key,
>> +               struct futex_hash_bucket *hb)
>> {
>>     drop_futex_key_refs(&q->key);
>>     get_futex_key_refs(key);
>>     q->key = *key;
>> +    q->lock_ptr = &hb->lock;
> 
> Hrm... turns out changing this breaks the 
> handle_early_requeue_pi_wakeup() logic.  I'll have to respin this patch 
> to account for that as well.  Please hold off on this patch.

In fact, this doesn't affect the handle_early_requeue_pi_wakeup() code 
in the slightest.  It only needs to hold a hb->lock (either one is 
adequate) to ensure the requeue routine has completed.  By changing the 
q->lock_ptr of the waiter to hb2->lock we ensure the pi_state is 
protected from concurrent access by futex_wait_requeue_pi() and new 
contending threads.

Ingo, please apply to tip/urgent.

Thanks,

-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team
--
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/