[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 34/70] x86/emul: CFI hardening


  • To: Andrew Cooper <amc96@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 15 Feb 2022 15:13:53 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RZR6gVge5Kk8yqJ9+685L/Ewmkuay5svUTOsyY0vD1Y=; b=gAxfqJ+MFkKx+D4p40q298tpbjotWPAR6TQ4RuLm0/xVU9m30J0uwH0Fl57Ln+OPLmAeA7u1NQDjo4JqUALNSlBwlzYQibsPWOhNbRPPOVhZLuRK6e7eWW6hQlSFnkRpnBaazhrHovuBOLt5O58v9HUpcCh9woLCU21XHImgb6213LaASkmnh1uwpHAuvb7D1AdTfHepQv/ui+V2XRc9GtR6C3kWI3I6kap1VZPS54UTtiMspAeUUHXaBgrGXtwYDZjPDCkDs4K2/+4yYT+fdZskwxUZs0qKRzj7IHcdI4I2dWZu7k7P193/SSayZPaNvzaCBpkZrX5KoEhUx8sA2A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AFtAFNiqgNYa4vfI11jTGM0YZ42E564If/ze1PWc56NHab+N2nyngyD5CoO6JhfOQ5xF999STUdDTzOuHK+xHUP/4SkjWj1V95+ELK0gDuWg2wAzRytNy3Q4m68jm20XkQtIbCVjREx1O3IF7IYZDCqDyd/QpnElX4sQdCUibfwCFsjD92BUxRT4Ul5hKOfRH9CQd/YwaUJM8FSMjlxms4uwvQ0JdRX5YDH4nzPmcc5aEH01sBnauPGYbQR5jOTfguoaKbrNzIS3MEuuDNe85H81mvrTlK+WnSo+QEkfOSEET9J/rgSht/TFyvwsQwOtNus/w/3ShPWEbGCgCaUozQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 15 Feb 2022 14:14:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 15.02.2022 14:43, Andrew Cooper wrote:
> On 14/02/2022 13:38, Jan Beulich wrote:
>> On 14.02.2022 13:50, Andrew Cooper wrote:
>>> Control Flow Integrity schemes use toolchain and optionally hardware support
>>> to help protect against call/jump/return oriented programming attacks.
>>>
>>> Use cf_check to annotate function pointer targets for the toolchain.
>>>
>>> pv_emul_is_mem_write() is only used in a single file.  Having it as a static
>>> inline is pointless because it can't be inlined to begin with.
>> I'd like you to consider to re-word this:
> 
> This is the reworded version.
> 
>> It being static inline was for
>> the case of there appearing a 2nd user. I don't view such as pointless.
> 
> I find that impossible to reconcile with your normal review feedback.

Interesting. I don't think I would have objected to something like
this, if it was conceivable that a 2nd user may appear. I don't
think this is the only inline function we've got with just a single
user. I also don't think this is the only inline function we've got
with its address taken, and hence having an out-of-line instantiation.

> It is unconditionally forced out of line because of how it's used,
> meaning that if it ever got used in a second translation unit we'd end
> up with a duplicate function, at which point it would need to be
> non-static and exported to pass review.  (And sanity.)

I'm afraid you've lost me here. What duplicate function? Before and
after the patch the function is static; what changes is merely the
"inline". Two CUs can have identically named static functions, can't
they? Or if that's not the point you try to make, then I have no idea
what it is that you're trying to tell me.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.