[SRU groovy 4/9] futex: Ensure the correct return value from futex_lock_pi()

[Krzysztof Kozlowski]

On 10/03/2021 11:40, Thadeu Lima de Souza Cascardo wrote:
> On Wed, Mar 10, 2021 at 10:29:14AM +0100, Krzysztof Kozlowski wrote:
>> On 09/03/2021 18:03, Thadeu Lima de Souza Cascardo wrote:
>>> From: Thomas Gleixner <tglx at linutronix.de>
>>>
>>> In case that futex_lock_pi() was aborted by a signal or a timeout and the
>>> task returned without acquiring the rtmutex, but is the designated owner of
>>> the futex due to a concurrent futex_unlock_pi() fixup_owner() is invoked to
>>> establish consistent state. In that case it invokes fixup_pi_state_owner()
>>> which in turn tries to acquire the rtmutex again. If that succeeds then it
>>> does not propagate this success to fixup_owner() and futex_lock_pi()
>>> returns -EINTR or -ETIMEOUT despite having the futex locked.
>>>
>>> Return success from fixup_pi_state_owner() in all cases where the current
>>> task owns the rtmutex and therefore the futex and propagate it correctly
>>> through fixup_owner(). Fixup the other callsite which does not expect a
>>> positive return value.
>>>
>>> Fixes: c1e2f0eaf015 ("futex: Avoid violating the 10th rule of futex")
>>> Signed-off-by: Thomas Gleixner <tglx at linutronix.de>
>>> Acked-by: Peter Zijlstra (Intel) <peterz at infradead.org>
>>> Cc: stable at vger.kernel.org
>>> (cherry picked from commit 12bb3f7f1b03d5913b3f9d4236a488aa7774dfe9)
>>> CVE-2021-3347
>>> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
>>> ---
>>>  kernel/futex.c | 31 ++++++++++++++++---------------
>>>  1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> Hi Thadeu,
>>
>> You reversed the order of 3/9 and 4/9 so their context is not exactly
>> the same. This should not matter, although raises the questions and
>> comparing patches with upstream is trickier.
>>
>> Best regards,
>> Krzysztof
> 
> Because they should have been reversed upstream, otherwise they don't build. At
> least, we keep some bisectability, though I find it hard that bisect will help
> with anything futex.

I see and it makes sense, although might be worth to mention this in
commit log.

Best regards,
Krzysztof