All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Cheng Jian <cj.chengjian@huawei.com>
Cc: tglx@linutronix.de, mingo@redhat.com, dvhart@infradead.org,
	linux-kernel@vger.kernel.org, xiexiuqi@huawei.com,
	huawei.libin@huawei.com
Subject: Re: [PATCH] futex: use fault_in to avoid infinite loop
Date: Wed, 6 Dec 2017 22:40:08 +0100	[thread overview]
Message-ID: <20171206214007.GI3857@worktop> (raw)
In-Reply-To: <20171206160400.yzewed5juhytfwyy@hirez.programming.kicks-ass.net>

On Wed, Dec 06, 2017 at 05:04:00PM +0100, Peter Zijlstra wrote:
> On Wed, Dec 06, 2017 at 10:21:07PM +0800, Cheng Jian wrote:
> > It will cause softlockup(infinite loop) in kernel
> > space when we use SYS_set_robust_list in futex which
> > incoming a misaligned address from user space.
> 
> Urgh, we should not allow that in the first place.
> 
> See how get_futex_key() does:
> 
>   if (unlikely(address % sizeof(u32)))
> 	return -EINVAL;
> 
> That same should also be true for the robust list. Using unaligned
> variables is insane.

Something a little like so perhaps..

---
Subject: futex: Sanitize user address in set_robust_list()

Passing in unaligned variables messes up cmpxchg on a whole bunch of
architectures. Also, not respecting the natural alignment of data
structures is pretty dumb to begin with.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/uapi/asm-generic/errno.h | 1 +
 kernel/futex.c                   | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/uapi/asm-generic/errno.h b/include/uapi/asm-generic/errno.h
index cf9c51ac49f9..4cb80d4ac160 100644
--- a/include/uapi/asm-generic/errno.h
+++ b/include/uapi/asm-generic/errno.h
@@ -119,5 +119,6 @@
 #define ERFKILL		132	/* Operation not possible due to RF-kill */
 
 #define EHWPOISON	133	/* Memory page has hardware error */
+#define EMORON		134	/* User did something particularly silly */
 
 #endif
diff --git a/kernel/futex.c b/kernel/futex.c
index 76ed5921117a..e2c1a818f88f 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -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;
+
 	current->robust_list = head;
 
 	return 0;

  reply	other threads:[~2017-12-06 21:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-06 14:21 [PATCH] futex: use fault_in to avoid infinite loop Cheng Jian
2017-12-06 16:04 ` Peter Zijlstra
2017-12-06 21:40   ` Peter Zijlstra [this message]
2017-12-08  5:21     ` Darren Hart
2017-12-08 10:50       ` Peter Zijlstra
2017-12-08 12:42     ` chengjian (D)
2017-12-28 14:21     ` [tip:locking/urgent] futex: Sanitize user address in set_robust_list() tip-bot for Peter Zijlstra
2017-12-30  7:40     ` [PATCH] futex: use fault_in to avoid infinite loop Michael Kerrisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171206214007.GI3857@worktop \
    --to=peterz@infradead.org \
    --cc=cj.chengjian@huawei.com \
    --cc=dvhart@infradead.org \
    --cc=huawei.libin@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=xiexiuqi@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.