From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753534AbbBESL4 (ORCPT ); Thu, 5 Feb 2015 13:11:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45041 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750891AbbBESLy (ORCPT ); Thu, 5 Feb 2015 13:11:54 -0500 Date: Thu, 5 Feb 2015 19:10:14 +0100 From: Oleg Nesterov To: Peter Zijlstra Cc: Darren Hart , Thomas Gleixner , Jerome Marchand , Larry Woodman , Mateusz Guzik , linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/1] futex: check PF_KTHREAD rather than !p->mm to filter out kthreads Message-ID: <20150205181014.GA20244@redhat.com> References: <20150202140515.GA26398@redhat.com> <20150202151159.GE26304@twins.programming.kicks-ass.net> <20150203200916.GA10545@redhat.com> <20150204111212.GF2896@worktop.programming.kicks-ass.net> <20150204202509.GA1502@redhat.com> <20150205162725.GK5029@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150205162725.GK5029@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Let me first say that I simply do not know if PI+robust futex is actually supposed (or guaranteed) to work. Documentation/pi-futex.txt says 'robustness' and 'PI' are two orthogonal properties of futexes, and all four combinations are possible: futex, robust-futex, PI-futex, robust+PI-futex. And exit_robust_list() checks bit 0 to detect the "PI" case, so I think this should work. However, this comment /* * This task is holding PI mutexes at exit time => bad. * Kernel cleans up PI-state, but userspace is likely hosed. * (Robust-futex cleanup is separate and might save the day for userspace.) */ above exit_pi_state_list() looks confusing. In fact it looks wrong if PI+robust should work. Because handle_futex_death() seems to rely on exit_pi_state_list. Now, if it should work, On 02/05, Peter Zijlstra wrote: > > So as long as we unhash _last_ I can't see this happening, we'll always > find the task, the robust list walk doesn't care about PI state. and it simply can't take care of PI state. ->pi_state can be NULL by the time exit_robust_list() is called. > But please, if you suspect, share a little more detail on how you see > this happening, this is not code I've looked at in detail before. Heh, I am reading it for the first time ;) So I can be easily wrong. But afaics the race/problem is very simple. Suppose a task T locks a PI+robust mutex and exits. I this case (I presume) sys_futex(uaddr, FUTEX_LOCK_PI) from another task X must always succeed sooner or later. But - X takes queue_lock() and reads *uaddr == T->pid. Need to setup pi_state and wait. FUTEX_WAITERS is set. - T exits and calls handle_futex_death(). This clears FUTEX_TID_MASK and sets FUTEX_OWNER_DIED, without any lock. T->pi_state_list is empty, exit_pi_state_list() does nothing. T goes away or simply sets PF_EXITPIDONE (lets ignore PF_EXITING). - X calls attach_to_pi_owner() and futex_find_get_task() returns NULL, or we detect PF_EXITPIDONE, this doesn't really matter. What does matter (unless I missed something) is that -ESRCH is wrong in this case. This mutex was unlocked. It is robust, so we should not miss this unlock. So I think that in this case we either need to recheck that *uaddr is still the same (and turn -ESRCH into -EAGAIN otherwise), or change handle_futex_death() to serialize with X so that it can proceed and attach pi_state. No? Oleg.