Re: [Xen-devel] [PATCH v3 15/15] argo: validate hypercall arg structures via compat machinery

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.01.19 at 08:22, <christopher.w.clark@xxxxxxxxx> wrote:
> Some details of the problem:
> 
> Without the macro overrides in place (ie. using the existing
> definitions) the build fails on CHECK_argo_send_addr  because this
> struct is defined with types that are themselves translated by the
> compat processing:

But that's a normal situation.

> typedef struct xen_argo_send_addr
> {
>     xen_argo_addr_t src;
>     xen_argo_addr_t dst;
> } xen_argo_send_addr_t;
> 
> compat/argo.c: In function '__checkFstruct_argo_send_addr__src':
> xen/include/xen/compat.h:170:18: error: comparison of distinct pointer
> types lacks a cast [-Werror]
>      return &x->f == &c->f; \
>                   ^
> xen/include/xen/compat.h:176:5: note: in expansion of macro
> 'CHECK_FIELD_COMMON_'
>      CHECK_FIELD_COMMON_(k, CHECK_NAME_(k, n ## __ ## f, F), n, f)
>      ^~~~~~~~~~~~~~~~~~~
> xen/include/compat/xlat.h:1238:5: note: in expansion of macro 'CHECK_FIELD_'
>      CHECK_FIELD_(struct, argo_send_addr, src); \
>      ^~~~~~~~~~~~
> compat/argo.c:43:1: note: in expansion of macro 'CHECK_argo_send_addr'
>  CHECK_argo_send_addr;
>  ^~~~~~~~~~~~~~~~~~~~
> 
> because xen_argo_addr_t is detected as a different type than
> compat_argo_addr_t -- when in practice is the same size and has the
> same fields at the same offsets.

Did you perhaps not add entries for the inner structures to xlat.lst?

>> > --- a/xen/common/Makefile
>> > +++ b/xen/common/Makefile
>> > @@ -70,7 +70,7 @@ obj-y += xmalloc_tlsf.o
>> >  obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo 
>> > unlz4 earlycpio,$(n).init.o)
>> >
>> >
>> > -obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o 
>> > multicall.o xlat.o)
>> > +obj-$(CONFIG_COMPAT) += $(addprefix compat/,argo.o domain.o kernel.o 
>> > memory.o multicall.o xlat.o)
>>
>> While a matter of taste to a certain degree, I'm not convinced
>> introducing a separate file for this is really necessary, especially
>> if some of the overrides to the CHECK_* macros would go away.
> 
> ack. I wouldn't have moved them out if the overrides weren't in use;
> but I will merge it into the implementation file if that is preferred.

Well - let's first see whether the overrides are really needed. If so,
keeping this in a separate file might indeed be better.

Jan



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