Subject: Re: [PATCH] futex: use fault_in to avoid infinite loop
To: Peter Zijlstra <peterz@infradead.org>
CC: <tglx@linutronix.de>, <mingo@redhat.com>, <dvhart@infradead.org>,
        <linux-kernel@vger.kernel.org>, <xiexiuqi@huawei.com>,
        <huawei.libin@huawei.com>, <dvhart@infradead.org>
References: <1512570067-79946-1-git-send-email-cj.chengjian@huawei.com>
 <20171206160400.yzewed5juhytfwyy@hirez.programming.kicks-ass.net>
 <20171206214007.GI3857@worktop>
From: "chengjian (D)" <cj.chengjian@huawei.com>
Message-ID: <f2828b10-43a3-5ce4-6e8e-e0464edb409b@huawei.com>
Date: Fri, 8 Dec 2017 20:42:28 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20171206214007.GI3857@worktop>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1363
Lines: 57


On 2017/12/7 5:40, Peter Zijlstra wrote:
> @@ -3262,6 +3262,8 @@ static int futex_wait_requeue_pi(u32 __user 
> *uaddr, unsigned int flags,
>   SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
>   		size_t, len)
>   {
> +	unsigned long address = (unsigned long)head;
> +
>   	if (!futex_cmpxchg_enabled)
>   		return -ENOSYS;
>   	/*
> @@ -3270,6 +3272,9 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
>   	if (unlikely(len != sizeof(*head)))
>   		return -EINVAL;
>   
> +	if (unlikely(address % __alignof__(*head)))
> +		return -EMORON;
> +

Yeah, This looks nicer. It solved the problem fundamentally
Also for other architecture, such as arm32 which will also
cause a crash without this PATCH.
If we incoming a misaligned address from user space,
the system call will return directly  with a new errno(EMORON).


BUT

	int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
	{
	retry:
		//......

		/* return -EFAULT */
         	if (cmpxchg_futex_value_locked (& nval, uaddr, uval, mval)) {
			/* always return 0 */
			if (fault_in_user_writeable(uaddr))
				return -1;	/* never here */
		goto retry; /* then goto retry */

		//......
	}

Does it correct here?
if we get other exception here next time, does kernel push himself into 
a new  infinite loop ?


Thanks.

CHENG Jian