Re: [Xen-devel] [PATCH v5 3/4] xen/x86: use SYMBOL when required

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.01.19 at 20:33, <sstabellini@xxxxxxxxxx> wrote:
> On Mon, 7 Jan 2019, Jan Beulich wrote:
>> >>> On 03.01.19 at 20:19, <sstabellini@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/alternative.c
>> > +++ b/xen/arch/x86/alternative.c
>> > @@ -194,7 +194,7 @@ void init_or_livepatch apply_alternatives(struct 
>> > alt_instr *start,
>> >       * So be careful if you want to change the scan order to any other
>> >       * order.
>> >       */
>> > -    for ( a = base = start; a < end; a++ )
>> > +    for ( a = base = start; (unsigned long)a < SYMBOL(end); a++ )
>> 
>> As said before, imo SYMBOL() should only eve be applied _directly_
>> to one of the (commonly linker script generated) symbols listed above,
>> never to any derivatives.
> 
> This is the most problematic case for doing that. This issue also
> highlights one of the problems mentioned in the thread about returning
> unsigned long or native type from SYMBOL.
> 
> To address your comment, I could do the following:
> 
>         apply_alternatives((struct alt_instr *)SYMBOL(__alt_instructions),
>                            (struct alt_instr 
> *)SYMBOL(__alt_instructions_end));
> 
> But then, we would still have two pointers pointing to different objects
> being compared in the implementation of apply_alternatives: `a' and
> `end' would be pointers pointing to __alt_instructions and
> __alt_instructions_end respectively. The MISRA-C violation would not be
> addressed, I think. The same thing would happen here, and in many other
> instances, if we switched SYMBOL to returning the native pointer type.
> 
> My preference would be to keep the code as-is in this patch and add an
> in-code comment in apply_alternatives to explain the behavior.

See my other reply (on the v4 thread): No matter what you do with
SYMBOL(), it is just a hiding mechanism. With that, it should be of
no interest to the validation where exactly the hiding occurs. Any
fully C language compliant solution can only involve replacement of
the "end" symbols by expressions evaluating to the size of a
particular object, _without_ subtracting start from end addresses.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel