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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 2 Feb 2018 17:11:02 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Nicolas Pitre <nico@...aro.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Jan Beulich <jbeulich@...e.com>,
        xen-devel <xen-devel@...ts.xenproject.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] xen: hypercall: fix out-of-bounds memcpy

On Fri, Feb 2, 2018 at 4:53 PM, Dan Carpenter <dan.carpenter@...cle.com> 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