Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c22d7613-fb2e-11ec-9544-a39ffc7fd955@intel.com>
Date:   Fri, 11 Aug 2023 12:13:30 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH RESEND] x86/pti: Fix kernel warnings for pti= and nopti
 cmdline options.
To:     Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>,
        <linux-kernel@vger.kernel.org>, <dave.hansen@linux.intel.com>,
        <luto@kernel.org>, <peterz@infradead.org>, <mingo@redhat.com>
CC:     <x86@kernel.org>, <bp@alien8.de>, <tglx@linutronix.de>,
        <hpa@zytor.com>
References: <20230808195615.9795-1-jo.vanbulck@cs.kuleuven.be>
 <c0aed6a4-23e0-d947-5c17-26e580d5c40a@intel.com>
 <405927c5-91bf-144d-a3d7-b82f5527f3c9@cs.kuleuven.be>
Content-Language: en-US
From:   Sohil Mehta <sohil.mehta@intel.com>
In-Reply-To: <405927c5-91bf-144d-a3d7-b82f5527f3c9@cs.kuleuven.be>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T1pvSlVzMmtQZENpdkpRZ2wybnFMT2pqQ29WNmlEU3VraTkzald1Y3dINkgr?=
 =?utf-8?B?bnRlK2x1Q0pVNTNKdXhzUkRyN0FXd2w4bTFyNnAySzFlVm5IakFCZG9DNkJw?=
 =?utf-8?B?VktKS2NDU3lSU1dlQWlrMkNXRms4QXhiQkljRkhPZzhCdFRXWEpRbXo4am10?=
 =?utf-8?B?dXdwcVJ6OXhOdktCQTdHcGx6SEMvaDRCOGgwTHVFR3RaQlFHak5SbW5NV2FK?=
 =?utf-8?B?QyttOEJ0enZ2cHc0SlhpTXc5TURvVHJPaEFXZ3h1OTNGT2RJYXdSRVZvc2hN?=
 =?utf-8?B?SDRIblUxazZFZmRlT1paalpIRm40WU1ZVk9Pd3lKeFBCU3BzZ2JUNUdpYXBw?=
 =?utf-8?B?NkdHSHB0MERNS0VnTnVWRk9KeDloM3JQU0tUWTdRWW85NHFBMXpWbktScitl?=
 =?utf-8?B?NGZ5NmdHMDAvRG1leU1KbGY1SmZTQUIvb2FnMGJlTy9OTXJoWVBHbVhHNGUx?=
 =?utf-8?B?aVR2QXVOdk80ZzRYMGxMQ1RxZGtjRXNveExhOFNSRkJMMmxtckd4MFh1TlZi?=
 =?utf-8?B?VGZOSDJiZUFZWDZKQm9CL3VTWWZIWmNDaHpvVWRZUERpakVKY24yN1cvN05V?=
 =?utf-8?B?ZXZNakNUQzAzU3R0aklTWG1xME15LzBQKzdRLzlRZ1hVKzJleklDaERBb3R4?=
 =?utf-8?B?ZmFvQTV1WS9MdEprbjZ4ZGtGWElmRUpxQU5pUHhvVG81alpBZ3V2YWpQd01L?=
 =?utf-8?B?TFd5WTEyeVJNR0RvNUdtQ2ZnVkFXbVM3MHlWcWtxWWlqOUpqTzhrcHh1bS8y?=
 =?utf-8?B?dElhTFoyUFN5SmsvRTBPNjJJbndDZnVYNkZRR3hEZjZnaEMzaW9oS20zUytO?=
 =?utf-8?B?d1ZhdXE0bjU1RkJUbVZVM3NzOHBXT3l2ajZwSTZCSTAzcDlBMitTM0owYzdB?=
 =?utf-8?B?cHJMUW8xbmZOMFVoYVZCc2dIOUtBUEVFQkFMRWVMSWFwN0JVT3o5TWQ4TWlZ?=
 =?utf-8?B?UlM4ZFdZSWhGQnVCS2ZvMWVMWi81RVdFeUFEbUEzS3BVRlZBSDlTVGROUFdV?=
 =?utf-8?B?TzJUT1RQa1hoNlplWG1aTkFVbjhDQ2hmQ2RYLzRHWHpVbHhUblNZQlJ5Ulpn?=
 =?utf-8?B?QS9hN3FuZFNMUlc2blEzNk5KSVZ6aStMZzQ4WFF1QjJiMGVpVHl2YnAvUTF2?=
 =?utf-8?B?bzliN2RMTVVWTmhhaGtMUGMyQlBWWVVEZXJhcmVpV2hzdlJHQjBobUMxdUIz?=
 =?utf-8?B?R1pwUCsxb0hYU0c0VUUxQXBvNVZIdXg5VXphazBkMFR1YzljLzlDZ1RiVDZC?=
 =?utf-8?B?QzRnSkxiL2pBU2krNXk4UkhIbVpHcWwzazBlOWRUTHhyQXpibCtsOWliTnRa?=
 =?utf-8?B?NXdRY3VOa0R6cHN6RVMyeWJUTUZQV1FVZ2N5QVVDOTkzSyt2Tm5vdlY3RFIr?=
 =?utf-8?B?aDFMM0RIVlJjcW5Qazh4SlplaUxkMS9ucEsvTFdvd1BhU0xLZjkxMm1CU3hX?=
 =?utf-8?B?eStuQnJpQUp2ZEQzTWhvWFNRQko0RUpWRjlYelFBcXB3OTlhM1UycjR4U285?=
 =?utf-8?B?Skd5aWlDajJhUVZBdjFTVmorNjVYb1lVdVdpaXNDNzZjQ0VNYS9UeHJUM0Rj?=
 =?utf-8?B?UFhQM3UrSnJDUG1ka09NWXFtNTA0TUxOazgxSytRcVY3SzFka1JQWXIzeldN?=
 =?utf-8?B?U0NHbUhMRStnYU5EMkMxNWsxcmhTdmZnVzZFQkxseGo1SXpiZzdjWDVWelVj?=
 =?utf-8?B?dTdXNHczS0xkZGpwSU1uS3d0Zk1DMWdkdzhCRjBxbHpUVFMxYWw4M0c3QVNF?=
 =?utf-8?B?MmRTU0paSk9jL0NQRHR5ejVmYXNUdC9CUW1abjBQMzBvcE9CTWFMVCs1MXZm?=
 =?utf-8?B?aWdERkg5ak01aTB5L0hScjVRWnBWUU1XdStaZVA0MTZEZFFvTEZKS2ZMMGVr?=
 =?utf-8?B?dnRnVC8ySERHc2c2ZGdaYW9BMTdESm1Pa1dlTzUxOVhMQU4wU09DcGdHOEor?=
 =?utf-8?B?bjVwc0JjK3lXQjhCejlYVHI2T3A0MDZVSjBJN2lqcEJFZndZSndqYnhYbVVD?=
 =?utf-8?B?WlpTb0hWek05amtjV0tGNUl5SG9tWGpQZkdWMEtoY2tXMXR5QmtjYXZWRXJz?=
 =?utf-8?B?U0pkMWJNc2xJYmxzY28vZVFFRU40cnVMUER0bU1YUTNCOGhzZmVnWGZMTTJk?=
 =?utf-8?Q?OHip6BeVsJLxUV1AK3OucBrnZ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 64f4152f-a49a-4b69-4013-08db9a9f0dd5
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3320.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Aug 2023 19:13:32.6915
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: f/nrrZcCYpdw5uo7CHCmsb4RTT/SopVfTVvmLYWb2rZPLpFkbzaiqSkYFvvOhMJDXeJYXbb56Rp/zqxxvaom9w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7634
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/11/2023 11:23 AM, Jo Van Bulck wrote:

> On 08.08.23 17:13, Sohil Mehta wrote:> Can mitigations be off through 
> some other mechanisms such as kernel config?
> I validated this both with and without the proposed patch.
> 

Great, thanks for checking that. The existing print is accurate then. If
it is printed "disabled on command line.", then PTI has been disabled
due to a command line option.

>> Maybe split the mitigations_off check into a separate if and it's own
>> unique print message?
>> Also, with the separated check you can avoid the unnecessary re-setting
>> of pti_mode when pti_mode == PTI_FORCE_OFF is true.
> 
> Thanks, makes sense. I'll make sure to do this in the next patch revision.
> 

Based on above, even when you split the if check only a single print
would be enough, right?

>> In the rare case that both pti= and nopti is set the existing code seems
>> to ignore the nopti option. Would the new implementation do the same?
> 
> Good point. In my understanding, passing such conflicting options is 
> undefined as per the specification [2] and I'm not sure if backwards 
> compatibility is a requirement?
> 

> That being said, I can see the argument that in this case of 
> security-sensitive functionality, it may be desirable to maintain 
> identical behavior for identical kernel parameter combinations and 
> sequences. 

I don't believe that is a requirement either. Sometimes kernel command
lines can get very long and people make mistakes. I just thought it is
neat that the current code is defaulting that way and we should probably
keep the same behavior since it makes sense.

> 
> --> I can update the patch to ensure backwards-compatible behavior in 
> both cases for the next patch revision.
> 

I agree, in both cases pti= overriding the other option (nopti or
mitigations=off) sounds reasonable to me.

Sohil