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

[Jan Beulich <jbeulich@xxxxxxxx>]

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