lkml.org 
[lkml]   [2018]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] xen: hypercall: fix out-of-bounds memcpy
On Mon, Feb 5, 2018 at 1:11 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Boris Ostrovsky
>> Sent: 02 February 2018 23:34
> ...
>> > switch (cmd) {
>> > + case EVTCHNOP_bind_interdomain:
>> > + len = sizeof(struct evtchn_bind_interdomain);
>> > + break;
>> > + case EVTCHNOP_bind_virq:
>> > + len = sizeof(struct evtchn_bind_virq);
>> > + break;
>> > + case EVTCHNOP_bind_pirq:
>> > + len = sizeof(struct evtchn_bind_pirq);
>> > + break;
>> > case EVTCHNOP_close:
>> > + len = sizeof(struct evtchn_close);
>> > + break;
>> > case EVTCHNOP_send:
>> > + len = sizeof(struct evtchn_send);
>> > + break;
>> > + case EVTCHNOP_alloc_unbound:
>> > + len = sizeof(struct evtchn_alloc_unbound);
>> > + break;
>> > + case EVTCHNOP_bind_ipi:
>> > + len = sizeof(struct evtchn_bind_ipi);
>> > + break;
>> > + case EVTCHNOP_status:
>> > + len = sizeof(struct evtchn_status);
>> > + break;
>> > case EVTCHNOP_bind_vcpu:
>> > + len = sizeof(struct evtchn_bind_vcpu);
>> > + break;
>> > case EVTCHNOP_unmask:
>> > - /* no output */
>> > + len = sizeof(struct evtchn_unmask);
>> > break;
>
> Are the EVTCHNOP_xxx values dense?
> In which case an array is almost certainly better than the switch statement.

They are, yes. PHYSDEVOP_xxx are also consecutive by start at '4'.
Dan made the same comment earlier, and I replied that my I had
considered it but went for the more failsafe route. I also verified my
assumption now that gcc in fact is smart enough to turn this
into a table by itself:

xen_physdev_op_compat:
pushq %r13 #
leal -4(%rdi), %eax #, _2
pushq %r12 #
pushq %rbp #
pushq %rbx #
subq $24, %rsp #,
# /git/arm-soc/drivers/xen/fallback.c:59: struct physdev_op op =
{ .cmd = cmd, };
movq $0, 4(%rsp) #, MEM[(struct physdev_op *)&op + 4B]
movq $0, 12(%rsp) #, MEM[(struct physdev_op *)&op + 4B]
movl $0, 20(%rsp) #, MEM[(struct physdev_op *)&op + 4B]
movl %edi, (%rsp) # cmd, op.cmd
cmpl $6, %eax #, _2
ja .L8 #,
movl %eax, %edi # _2, _2
# /git/arm-soc/drivers/xen/fallback.c:87: memcpy(&op.u, arg, len);
leaq 8(%rsp), %r12 #, tmp98
movq %rsi, %rbx # arg, arg
movq CSWTCH.17(,%rdi,8), %r13 # CSWTCH.17, _5
movq %r12, %rdi # tmp98,
movq %r13, %rdx # _5,
call __memcpy #
# /git/arm-soc/drivers/xen/fallback.c:88: rc = _hypercall1(int,
physdev_op_compat, &op);
movq %rsp, %rdi #, __arg1
#APP
# 88 "/git/arm-soc/drivers/xen/fallback.c" 1
call hypercall_page+608 #


Arnd

\
 
 \ /
  Last update: 2018-02-05 13:37    [W:0.057 / U:36.524 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site