From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752774AbbC0RbI (ORCPT ); Fri, 27 Mar 2015 13:31:08 -0400 Received: from mail.skyhub.de ([78.46.96.112]:55837 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752353AbbC0RbE (ORCPT ); Fri, 27 Mar 2015 13:31:04 -0400 Date: Fri, 27 Mar 2015 18:29:14 +0100 From: Borislav Petkov To: Dave Hansen Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@linutronix.de, dave.hansen@linux.intel.com Subject: Re: [PATCH 15/17] x86, mpx: do 32-bit-only cmpxchg for 32-bit apps Message-ID: <20150327172914.GE5517@pd.tnic> References: <20150326183327.64807530@viggo.jf.intel.com> <20150326183353.A2A5B371@viggo.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150326183353.A2A5B371@viggo.jf.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 26, 2015 at 11:33:53AM -0700, Dave Hansen wrote: > > From: Dave Hansen > > user_atomic_cmpxchg_inatomic() actually looks at sizeof(*ptr) to > figure out how many bytes to copy. If we run it on a 64-bit > kernel with a 64-bit pointer, it will copy a 64-bit bounds > directory entry. That's fine, except when we have 32-bit > programs with 32-bit bounds directory entries and we only *want* > 32-bits. > > This patch breaks the cmpxchg operation out in to its own > function and performs the 32-bit type swizzling in there. > > Note, the "64-bit" version of this code _would_ work on a > 32-bit-only kernel. The issue this patch addresses is only for > when the kernel's 'long' is mismatched from the size of the > bounds directory entry of the process we are working on. > > The new helper modifies 'actual_old_val' or returns an error. > But gcc doesn't know this, so it warns about 'actual_old_val' > being unused. Shut it up with an uninitialized_var(). > > Signed-off-by: Dave Hansen > --- > > b/arch/x86/mm/mpx.c | 41 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 5 deletions(-) > > diff -puN arch/x86/mm/mpx.c~mpx-variable-sized-userspace-pokes arch/x86/mm/mpx.c > --- a/arch/x86/mm/mpx.c~mpx-variable-sized-userspace-pokes 2015-03-26 11:27:33.927520821 -0700 > +++ b/arch/x86/mm/mpx.c 2015-03-26 11:27:33.931521001 -0700 > @@ -421,6 +421,35 @@ int mpx_disable_management(struct task_s > return 0; > } > > +static int mpx_cmpxchg_bd_entry(struct mm_struct *mm, > + unsigned long *actual_old_val_ptr, long __user *bd_entry_addr, > + unsigned long expected_old_val, unsigned long new_bd_entry) > +{ > + int ret; > + /* > + * user_atomic_cmpxchg_inatomic() actually uses sizeof() > + * the pointer thatt we pass to it to figure out how much > + * data to cmpxchg. We have to be careful here not to > + * pass a pointer to a 64-bit data type when we only want > + * a 32-bit copy. > + */ > + if (is_64bit_mm(mm)) { > + ret = user_atomic_cmpxchg_inatomic(actual_old_val_ptr, > + bd_entry_addr, expected_old_val, new_bd_entry); > + } else { > + u32 uninitialized_var(actual_old_val_32); > + u32 expected_old_val_32 = expected_old_val; > + u32 new_bd_entry_32 = new_bd_entry; > + u32 __user *bd_entry_32 = (u32 __user *)bd_entry_addr; > + ret = user_atomic_cmpxchg_inatomic(&actual_old_val_32, > + bd_entry_32, expected_old_val_32, > + new_bd_entry_32); Hmm, I would've added a user_atomic_cmpxchg_inatomic_size() macro which calls __user_atomic_cmpxchg_inatomic(). -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --