Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750912AbdL3HlI (ORCPT ); Sat, 30 Dec 2017 02:41:08 -0500 Received: from mail-qk0-f195.google.com ([209.85.220.195]:33897 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbdL3HlG (ORCPT ); Sat, 30 Dec 2017 02:41:06 -0500 X-Google-Smtp-Source: ACJfBov6NKGWWC8VNVgzgPv72afZYTzmKziCLJD2NS1jsJZuTOl3w3emAJTqun8wRXBnjYAvtfFMV0u+OxklgIGR6Q0= MIME-Version: 1.0 In-Reply-To: <20171206214007.GI3857@worktop> References: <1512570067-79946-1-git-send-email-cj.chengjian@huawei.com> <20171206160400.yzewed5juhytfwyy@hirez.programming.kicks-ass.net> <20171206214007.GI3857@worktop> From: Michael Kerrisk Date: Sat, 30 Dec 2017 08:40:45 +0100 X-Google-Sender-Auth: G2cMV-k0tEYS5wJ55B6qboIzjh8 Message-ID: Subject: Re: [PATCH] futex: use fault_in to avoid infinite loop To: Peter Zijlstra Cc: Cheng Jian , Thomas Gleixner , Ingo Molnar , dvhart@infradead.org, Linux Kernel , xiexiuqi@huawei.com, huawei.libin@huawei.com, Randy Dunlap , Michael Kerrisk Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2819 Lines: 82 Peter, On Wed, Dec 6, 2017 at 10:40 PM, Peter Zijlstra wrote: > 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) > --- > 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; > + Do we really need to make these sorts of minor insults to user-space programmers? Can we make this -EINVAL, please? (EINVAL in the standard error for misaligned on calls such as mmap(), mremap(), clone(), read(), write(), seccomp(), shmat(), and **other futex() operations**.) Thanks, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Author of "The Linux Programming Interface", http://blog.man7.org/