Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752731AbcD2UcT (ORCPT ); Fri, 29 Apr 2016 16:32:19 -0400 Received: from mga11.intel.com ([192.55.52.93]:48203 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbcD2UcS (ORCPT ); Fri, 29 Apr 2016 16:32:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,553,1455004800"; d="scan'208";a="965630095" Subject: Re: [PATCH v4 10/10] x86/xsaves: Re-enable XSAVES To: Yu-cheng Yu , x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org References: Cc: Andy Lutomirski , Borislav Petkov , Sai Praneeth Prakhya , "Ravi V. Shankar" , Fenghua Yu From: Dave Hansen Message-ID: <5723C4CF.3050209@linux.intel.com> Date: Fri, 29 Apr 2016 13:32:15 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1262 Lines: 26 On 03/04/2016 10:12 AM, Yu-cheng Yu wrote: > We did not handle XSAVES* instructions correctly. There were issues in > converting between standard and compacted format when interfacing with > user-space. These issues have been corrected. > > Add a WARN_ONCE() to make it clear that XSAVES supervisor states are not > yet implemented. The reason I haven't acked this patch is that I want to be _sure_ that we've audited all of the call paths that access the XSAVE buffer to ensure that they can all either handle the XSAVES format *or* don't care for whatever reason. Could you share the steps that you've taken to assure yourself that all of the call paths are handled and we don't have more bugs? FWIW, this was the single biggest lesson I learned from the failure the last time this got turned on: we simply didn't go look for all the places that the new format had to be handled. Let's be sure we don't repeat that. If we get this *wrong* in another user/kernel interface like we did for ptrace and the signal save/restore and we ever enable a supervisor state we've got an almost certain immediate root hole of some kind. I think we need to exercise some serious caution here. Thank $DEITY we don't have any supported supervisor states at the moment.