Re: [Xen-devel] [PATCH v3 3/3] altp2m: Implement p2m_get_mem_access for altp2m views

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.16 at 18:32, <tlengyel@xxxxxxxxxxx> wrote:
> On Tue, Feb 9, 2016 at 10:17 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
>> >>> On 05.02.16 at 22:22, <tlengyel@xxxxxxxxxxx> wrote:
>> > --- a/xen/include/public/memory.h
>> > +++ b/xen/include/public/memory.h
>> > @@ -423,11 +423,20 @@ struct xen_mem_access_op {
>> >      /* xenmem_access_t */
>> >      uint8_t access;
>> >      domid_t domid;
>> > -    /*
>> > -     * Number of pages for set op
>> > -     * Ignored on setting default access and other ops
>> > -     */
>> > -    uint32_t nr;
>> > +    union {
>> > +        /*
>> > +         * Number of pages for set op
>> > +         * Ignored on setting default access and other ops
>> > +         */
>> > +        uint32_t nr;
>> > +        /*
>> > +         * altp2m id used for get op, ignored for other ops
>> > +         */
>> > +        struct altp2m {
>> > +            uint16_t idx;
>> > +            uint16_t pad;
>> > +        } altp2m;
>> > +    } u;
>>
>> Am I overlooking something, or is this new padding field not being
>> checked to be zero on input (allowing seamless use for an actual
>> purpose later on)? It's a tools-only interface (i.e. not considered
>> stable), but anyway...
> 
> There is no plan to use it for anything in this context in the future so we
> could enforce it being zero. However, even if someone erroneously used the
> nr field and set some arbitrary large number in there, the altp2m idx is
> checked to be sure it's not larger then the max supported, so IMHO there is
> enough error checking on it in place already.

Looks like you didn't understand: Whatever padding fields we have
in the public interface structures, when we don't enforce them to
be set to zero by callers, we can't _later_ assign meaning to these
fields without possibly breaking existing code. This is less of a
problem for domctls and sysctls, since they're versioned, and not
as bad an issue as it would be for general consumption interfaces,
but should imo nevertheless be avoided.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel