Re: [PATCH] xen: hypercall: fix out-of-bounds memcpy

[Arnd Bergmann]

On Fri, Feb 2, 2018 at 4:53 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Fri, Feb 02, 2018 at 04:32:31PM +0100, Arnd Bergmann wrote:

>> --- a/drivers/xen/fallback.c
>> +++ b/drivers/xen/fallback.c
>> @@ -7,75 +7,87 @@
>>
>>  int xen_event_channel_op_compat(int cmd, void *arg)
>>  {
>> -     struct evtchn_op op;
>> +     struct evtchn_op op = { .cmd = cmd, };
>> +     size_t len;
>>       int rc;
>>
>> -     op.cmd = cmd;
>> -     memcpy(&op.u, arg, sizeof(op.u));
>> -     rc = _hypercall1(int, event_channel_op_compat, &op);
>> -
>>       switch (cmd) {
>> +     case EVTCHNOP_bind_interdomain:
>> +             len = sizeof(struct evtchn_bind_interdomain);
>> +             break;
>
> This was in the original code, but I'm slightly surpprised that we're
> using a switch statement here instead of a table.  I would have thought
> this is a fast path but I don't know xen at all.

I thought about using a table, but figured the switch statement
had a lower risk of getting something slightly wrong during the
conversion.

I would expect gcc to turn this into a table lookup, since all the
constants are consecutive, but it should not really matter since
this is only the fallback path for ancient Xen releases. When Xen
guest support was first merged in 2007, it was already
deprecated.

      Arnd