linux-kernel - Re: [PATCH] kernel/futex.c: notice the return value after rt_mutex_finish_proxy

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:	Tue, 03 Sep 2013 13:10:25 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Darren Hart <dvhart@...ux.intel.com>
CC:	Thomas Gleixner <tglx@...utronix.de>, ccross@...roid.com,
	Mel Gorman <mgorman@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel/futex.c: notice the return value after rt_mutex_finish_proxy_lock()
 fails

Hello Maintainers:

Please help check this patch, when you have time.


Thanks.

On 08/21/2013 11:48 AM, Chen Gang wrote:
> On 08/21/2013 12:19 AM, Darren Hart wrote:
>> HopingOn Tue, 2013-08-20 at 11:07 +0800, Chen Gang wrote:
>>
>>
>> Hi Chen,
>>
>>> rt_mutex_finish_proxy_lock() can return failure code (e.g. -EINTR,
>>> -ETIMEDOUT).
>>>
>>> Original implementation has already noticed about it, but not check it
>>> before next work.
>>>
>>> Also let coments within 80 columns to pass "./scripts/checkpatch.pl".
>>>
>>>
>>> Signed-off-by: Chen Gang <gang.chen@...anux.com>
>>> ---
>>>  kernel/futex.c |   30 ++++++++++++++++--------------
>>>  1 files changed, 16 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/kernel/futex.c b/kernel/futex.c
>>> index c3a1a55..1a94e7d 100644
>>> --- a/kernel/futex.c
>>> +++ b/kernel/futex.c
>>> @@ -2373,21 +2373,23 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
>>>  		ret = rt_mutex_finish_proxy_lock(pi_mutex, to, &rt_waiter, 1);
>>>  		debug_rt_mutex_free_waiter(&rt_waiter);
>>>  
>>> -		spin_lock(q.lock_ptr);
>>> -		/*
>>> -		 * Fixup the pi_state owner and possibly acquire the lock if we
>>> -		 * haven't already.
>>> -		 */
>>> -		res = fixup_owner(uaddr2, &q, !ret);
>>
>>
>> This call catches a corner case which appears to be skipped now. Or am I
>> missing how you accounted for that?
>>
>>
> 
> Pardon ?
> 
> Hmm... this patch lets related code block in "if(!ret) {...}", should
> not remove any code.
> 
> Please help check again for whether what I have done is correct or not.
> 
> Thanks.
> 
>>> -		/*
>>> -		 * If fixup_owner() returned an error, proprogate that.  If it
>>> -		 * acquired the lock, clear -ETIMEDOUT or -EINTR.
>>> -		 */
>>> -		if (res)
>>> -			ret = (res < 0) ? res : 0;
>>> +		if (!ret) {
>>> +			spin_lock(q.lock_ptr);
>>> +			/*
>>> +			 * Fixup the pi_state owner and possibly acquire the
>>> +			 * lock if we haven't already.
>>> +			 */
>>> +			res = fixup_owner(uaddr2, &q, !ret);
>>> +			/*
>>> +			 * If fixup_owner() returned an error, proprogate that.
>>> +			 * If it acquired the lock, clear -ETIMEDOUT or -EINTR.
>>> +			 */
>>> +			if (res)
>>> +				ret = (res < 0) ? res : 0;
>>>  
>>> -		/* Unqueue and drop the lock. */
>>> -		unqueue_me_pi(&q);
>>> +			/* Unqueue and drop the lock. */
>>> +			unqueue_me_pi(&q);
>>> +		}
>>>  	}
>>>  
>>>  	/*
>>
>> Thanks,
>>
> 
> 


-- 
Chen Gang
--
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/