From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 330231F78E6; Fri, 5 Jun 2026 09:50:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780653045; cv=none; b=qg4p8j8EV15slP6eutnXe+xBchQO/DDVMY67YriI/edYYOw/PbRee6S3++Pe5KkQk4ldGNf3w0efvKy0ydsicyNsTjWG3hIMHoN9fOpRwDhBEkyP9+Q27fWxouzxUgZ2IJDVwas60qN4kEPr9bUNA7OAGTSkpSjlITnhX1gM0cM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780653045; c=relaxed/simple; bh=P3yEI2e9RXJ2+V6UBcaTjSB7kb4ua4bdFeiCZWpMeNo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KuUBOO57Eyl3zbjERLy+zkaDKzJVNN1w5QSrQ+J0xOa7Exq5fxlCHSSHkx7lIFDdJQ9TSD/cRatkYbReIPLBPL6/xwBMWyVqhGryt+lAMYxCfHEWiqj9TYmTNXo1bal7jfIn7aQ59P0SUA+j37YPYuZ1tdcPX4eXMi/nxNXJ4qQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jT7sKdkb; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jT7sKdkb" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 73C011F00893; Fri, 5 Jun 2026 09:50:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780653043; bh=e8X/++vp/UllliUOSKrXQ+yZZ/YtDyV3wUQlvofIjG8=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=jT7sKdkbWYnzLaD+AvOK9qWJfkHJJi3ycX+H9+Ayl9YWiM1KU5WjkjLbmpfKEV91Z n95G+0y4KlCtTzfHx11SEsMf8C76IIqoBFR58GIXvEGdaTi7gPoiymGgAqBrhIHxSJ axidY5x+VFvINKPytBa3cOgTuBfgXoTXl4n9FlIjja8bY+6WUu9yQM+FogVtMbv+z2 AsKk/YmaCXzS6jbMg7LcEL6LtawGGgBv4kuRFVVOftQfH7GehspIxQpvjrj8XUcUmt HHuamU9lFaGXNouehUQzjcV8mTV6AM8P5/rWBt3Q996UkYUUGahvNLruwDdgBqd9Dl 4NhdAH6BCv0cQ== Date: Fri, 5 Jun 2026 11:50:39 +0200 From: Ingo Molnar To: Andrei Vagin Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , "Chang S. Bae" , linux-kernel@vger.kernel.org, criu@lists.linux.dev, Dave Hansen , x86@kernel.org, "H. Peter Anvin" Subject: Re: [PATCH 3/4] x86/fpu: Add consistency check between xstate_size and xfeatures Message-ID: References: <20260604165604.1195243-1-avagin@google.com> <20260604165604.1195243-4-avagin@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260604165604.1195243-4-avagin@google.com> * Andrei Vagin wrote: > The signal frame is designed to be self-describing, where xstate_size > indicates the actual size of the xstate context. The kernel previously > lacked a check to ensure that the provided xstate_size was sufficient > for the features enabled in the xfeatures mask. Additionally, > restore_fpregs_from_user() always used the default xstate_size to fault > in the xstate user buffer. > > These consistency checks have been added: > * Validate that xfeatures is a subset of the features enabled for the > task. > * Calculate the required size for the validated xfeatures mask. > * Ensure the provided xstate_size is sufficient. > > These checks prevent the kernel from attempting to fault in memory past > the end of a frame. > > Signed-off-by: Andrei Vagin > --- > arch/x86/kernel/fpu/signal.c | 29 +++++++++++++++++++++++------ > arch/x86/kernel/fpu/xstate.c | 2 +- > arch/x86/kernel/fpu/xstate.h | 2 ++ > 3 files changed, 26 insertions(+), 7 deletions(-) So since this is a potentially invasive change, could you please split it up into further incremental steps, with the behavioral changes at the end: x86/fpu: Export xstate_calculate_size() internally x86/fpu: Extend restore_fpregs_from_user() with 'xstate_size' x86/fpu: Rename 'fpstate' to 'sig_fpstate' in check_xstate_in_sigframe() x86/fpu: Introduce 'fpstate' helper variable in check_xstate_in_sigframe() etc. To make it all more reviewable & bisectable. Also, could the check_xstate_in_sigframe() function get some TLC before we modify it materially: 1) arch/x86/kernel/fpu/signal.c:static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf, arch/x86/kernel/fpu/signal.c: if (!check_xstate_in_sigframe(buf_fx, &fx_sw_user)) Please harmonize the argument names: why is it 'buf_fx' in one function and 'fxbuf' in another? We should probably standardize on 'buf_fx' everywhere. 2) Why is an error condition label called 'setfx'? How about 'error_setfx' or so. Makes patches more straightforward to read. etc., I'm sure there's more. Plus regarding the behavioral changes, this one should probably be its own patch as well: + /* Enforce XFEATURE_MASK_FPSSE when XSAVE is enabled */ + fx_sw->xfeatures |= XFEATURE_MASK_FPSSE; With the new size checks in another patch, right? Thanks, Ingo