From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751994AbdA3RTv (ORCPT ); Mon, 30 Jan 2017 12:19:51 -0500 Received: from mga06.intel.com ([134.134.136.31]:3740 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750836AbdA3RTt (ORCPT ); Mon, 30 Jan 2017 12:19:49 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,312,1477983600"; d="scan'208";a="1119797138" Date: Mon, 30 Jan 2017 09:11:59 -0800 From: Yu-cheng Yu To: Ingo Molnar Cc: linux-kernel@vger.kernel.org, Andrew Morton , Andy Lutomirski , Borislav Petkov , Dave Hansen , Fenghua Yu , "H . Peter Anvin" , Linus Torvalds , Oleg Nesterov , Peter Zijlstra , Rik van Riel , Thomas Gleixner Subject: Re: [PATCH 09/14] x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*() Message-ID: <20170130171159.GA27534@test-lenovo> References: <1485426179-13681-1-git-send-email-mingo@kernel.org> <1485426179-13681-10-git-send-email-mingo@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1485426179-13681-10-git-send-email-mingo@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 26, 2017 at 11:22:54AM +0100, Ingo Molnar wrote: > 'size_total' is derived from an unsigned input parameter - and then converted > to 'int' and checked for negative ranges: > > if (size_total < 0 || offset < size_total) { > > This conversion and the checks are unnecessary obfuscation, reject overly > large requested copy sizes outright and simplify the underlying code. > > Reported-by: Rik van Riel > Cc: Andy Lutomirski > Cc: Borislav Petkov > Cc: Dave Hansen > Cc: Fenghua Yu > Cc: H. Peter Anvin > Cc: Linus Torvalds > Cc: Oleg Nesterov > Cc: Thomas Gleixner > Cc: Yu-cheng Yu > Cc: Fenghua Yu > Signed-off-by: Ingo Molnar > --- > arch/x86/kernel/fpu/xstate.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c > index 8f9da89015e6..cceabca485c8 100644 > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -924,15 +924,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > * the source data pointer or increment pos, count, kbuf, and ubuf. > */ > static inline int > -__copy_xstate_to_kernel(void *kbuf, > - const void *data, > - unsigned int offset, unsigned int size, int size_total) > +__copy_xstate_to_kernel(void *kbuf, const void *data, > + unsigned int offset, unsigned int size, unsigned int size_total) > { > - if (!size) > - return 0; > - > - if (size_total < 0 || offset < size_total) { > - unsigned int copy = size_total < 0 ? size : min(size, size_total - offset); > + if (offset < size_total) { > + unsigned int copy = min(size, size_total - offset); > > memcpy(kbuf + offset, data, copy); > } > @@ -985,12 +981,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of > offset = xstate_offsets[i]; > size = xstate_sizes[i]; > > + /* The next component has to fit fully into the output buffer: */ > + if (offset + size > size_total) > + break; This makes sense, but would be different from the non-compacted format path where this rule is not enforced. Do we want to unify both? Yu-cheng