Konrad Rzeszutek Wilk
2012-Oct-29 14:08 UTC
[PATCH v1] Bug-fixes for v3.7 back-ported from Jan''s postings.
These are back-ports from the ones that Jan posted. Please take a look - I might have missed an email about one of the patches and I can''t find it my mailbox :-( Either way, was thinking to put these in v3.7. arch/x86/include/asm/xen/hypercall.h | 21 +++----- drivers/xen/Makefile | 2 +- drivers/xen/fallback.c | 78 ++++++++++++++++++++++++++++++ drivers/xen/xenbus/xenbus_dev_frontend.c | 2 +- 4 files changed, 87 insertions(+), 16 deletions(-) Jan Beulich (2): xen/hypercall: fix hypercall fallback code for very old hypervisors xen/xenbus: fix overflow check in xenbus_file_write()
Konrad Rzeszutek Wilk
2012-Oct-29 14:08 UTC
[PATCH 1/2] xen/hypercall: fix hypercall fallback code for very old hypervisors
From: Jan Beulich <jbeulich@suse.com> While copying the argument structures in HYPERVISOR_event_channel_op() and HYPERVISOR_physdev_op() into the local variable is sufficiently safe even if the actual structure is smaller than the container one, copying back eventual output values the same way isn''t: This may collide with on-stack variables (particularly "rc") which may change between the first and second memcpy() (i.e. the second memcpy() could discard that change). Move the fallback code into out-of-line functions, and handle all of the operations known by this old a hypervisor individually: Some don''t require copying back anything at all, and for the rest use the individual argument structures'' sizes rather than the container''s. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> [v2: Reduce #define/#undef usage in HYPERVISOR_physdev_op_compat().] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/include/asm/xen/hypercall.h | 21 +++------ drivers/xen/Makefile | 2 +- drivers/xen/fallback.c | 78 ++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 15 deletions(-) create mode 100644 drivers/xen/fallback.c diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h index 59c226d..c812360 100644 --- a/arch/x86/include/asm/xen/hypercall.h +++ b/arch/x86/include/asm/xen/hypercall.h @@ -359,18 +359,14 @@ HYPERVISOR_update_va_mapping(unsigned long va, pte_t new_val, return _hypercall4(int, update_va_mapping, va, new_val.pte, new_val.pte >> 32, flags); } +int __must_check HYPERVISOR_event_channel_op_compat(int, void *); static inline int HYPERVISOR_event_channel_op(int cmd, void *arg) { int rc = _hypercall2(int, event_channel_op, cmd, arg); - if (unlikely(rc == -ENOSYS)) { - struct evtchn_op op; - op.cmd = cmd; - memcpy(&op.u, arg, sizeof(op.u)); - rc = _hypercall1(int, event_channel_op_compat, &op); - memcpy(arg, &op.u, sizeof(op.u)); - } + if (unlikely(rc == -ENOSYS)) + rc = HYPERVISOR_event_channel_op_compat(cmd, arg); return rc; } @@ -386,17 +382,14 @@ HYPERVISOR_console_io(int cmd, int count, char *str) return _hypercall3(int, console_io, cmd, count, str); } +int __must_check HYPERVISOR_physdev_op_compat(int, void *); + static inline int HYPERVISOR_physdev_op(int cmd, void *arg) { int rc = _hypercall2(int, physdev_op, cmd, arg); - if (unlikely(rc == -ENOSYS)) { - struct physdev_op op; - op.cmd = cmd; - memcpy(&op.u, arg, sizeof(op.u)); - rc = _hypercall1(int, physdev_op_compat, &op); - memcpy(arg, &op.u, sizeof(op.u)); - } + if (unlikely(rc == -ENOSYS)) + rc = HYPERVISOR_physdev_op_compat(cmd, arg); return rc; } diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 0e863703..46de6cd 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,7 +2,7 @@ ifneq ($(CONFIG_ARM),y) obj-y += manage.o balloon.o obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o endif -obj-y += grant-table.o features.o events.o +obj-y += grant-table.o features.o events.o fallback.o obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) diff --git a/drivers/xen/fallback.c b/drivers/xen/fallback.c new file mode 100644 index 0000000..0bdc468 --- /dev/null +++ b/drivers/xen/fallback.c @@ -0,0 +1,78 @@ +#include <linux/kernel.h> +#include <linux/string.h> +#include <linux/bug.h> +#include <asm/hypervisor.h> +#include <asm/xen/hypercall.h> + +int HYPERVISOR_event_channel_op_compat(int cmd, void *arg) +{ + struct evtchn_op op; + int rc; + + op.cmd = cmd; + memcpy(&op.u, arg, sizeof(op.u)); + rc = _hypercall1(int, event_channel_op_compat, &op); + + switch (cmd) { + case EVTCHNOP_close: + case EVTCHNOP_send: + case EVTCHNOP_bind_vcpu: + case EVTCHNOP_unmask: + /* no output */ + break; + +#define COPY_BACK(eop) \ + case EVTCHNOP_##eop: \ + memcpy(arg, &op.u.eop, sizeof(op.u.eop)); \ + break + + COPY_BACK(bind_interdomain); + COPY_BACK(bind_virq); + COPY_BACK(bind_pirq); + COPY_BACK(status); + COPY_BACK(alloc_unbound); + COPY_BACK(bind_ipi); +#undef COPY_BACK + + default: + WARN_ON(rc != -ENOSYS); + break; + } + + return rc; +} + +int HYPERVISOR_physdev_op_compat(int cmd, void *arg) +{ + struct physdev_op op; + int rc; + + op.cmd = cmd; + memcpy(&op.u, arg, sizeof(op.u)); + rc = _hypercall1(int, physdev_op_compat, &op); + + switch (cmd) { + case PHYSDEVOP_IRQ_UNMASK_NOTIFY: + case PHYSDEVOP_set_iopl: + case PHYSDEVOP_set_iobitmap: + case PHYSDEVOP_apic_write: + /* no output */ + break; + +#define COPY_BACK(pop, fld) \ + case PHYSDEVOP_##pop: \ + memcpy(arg, &op.u.fld, sizeof(op.u.fld)); \ + break + + COPY_BACK(irq_status_query, irq_status_query); + COPY_BACK(apic_read, apic_op); + COPY_BACK(ASSIGN_VECTOR, irq_op); +#undef COPY_BACK + + default: + WARN_ON(rc != -ENOSYS); + break; + } + + return rc; +} -- 1.7.7.6
Konrad Rzeszutek Wilk
2012-Oct-29 14:08 UTC
[PATCH 2/2] xen/xenbus: fix overflow check in xenbus_file_write()
From: Jan Beulich <JBeulich@suse.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> [v1: Rebased on upstream] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/xen/xenbus/xenbus_dev_frontend.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index 89f7625..ac72702 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c @@ -458,7 +458,7 @@ static ssize_t xenbus_file_write(struct file *filp, goto out; /* Can''t write a xenbus message larger we can buffer */ - if ((len + u->len) > sizeof(u->u.buffer)) { + if (len > sizeof(u->u.buffer) - u->len) { /* On error, dump existing buffer */ u->len = 0; rc = -EINVAL; -- 1.7.7.6
Konrad Rzeszutek Wilk
2012-Oct-30 15:44 UTC
Re: [PATCH 1/2] xen/hypercall: fix hypercall fallback code for very old hypervisors
On Mon, Oct 29, 2012 at 10:08:17AM -0400, Konrad Rzeszutek Wilk wrote:> From: Jan Beulich <jbeulich@suse.com> > > While copying the argument structures in HYPERVISOR_event_channel_op() > and HYPERVISOR_physdev_op() into the local variable is sufficiently > safe even if the actual structure is smaller than the container one, > copying back eventual output values the same way isn''t: This may > collide with on-stack variables (particularly "rc") which may change > between the first and second memcpy() (i.e. the second memcpy() could > discard that change). > > Move the fallback code into out-of-line functions, and handle all of > the operations known by this old a hypervisor individually: Some don''t > require copying back anything at all, and for the rest use the > individual argument structures'' sizes rather than the container''s. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > [v2: Reduce #define/#undef usage in HYPERVISOR_physdev_op_compat().] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>And it looks like I get ERROR: "HYPERVISOR_event_channel_op_compat" [drivers/xen/xen-evtchn.ko] undefined! when I build xen-evtchn as module. Jan did you encounter this issue on 2.6.18?> --- > arch/x86/include/asm/xen/hypercall.h | 21 +++------ > drivers/xen/Makefile | 2 +- > drivers/xen/fallback.c | 78 ++++++++++++++++++++++++++++++++++ > 3 files changed, 86 insertions(+), 15 deletions(-) > create mode 100644 drivers/xen/fallback.c > > diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h > index 59c226d..c812360 100644 > --- a/arch/x86/include/asm/xen/hypercall.h > +++ b/arch/x86/include/asm/xen/hypercall.h > @@ -359,18 +359,14 @@ HYPERVISOR_update_va_mapping(unsigned long va, pte_t new_val, > return _hypercall4(int, update_va_mapping, va, > new_val.pte, new_val.pte >> 32, flags); > } > +int __must_check HYPERVISOR_event_channel_op_compat(int, void *); > > static inline int > HYPERVISOR_event_channel_op(int cmd, void *arg) > { > int rc = _hypercall2(int, event_channel_op, cmd, arg); > - if (unlikely(rc == -ENOSYS)) { > - struct evtchn_op op; > - op.cmd = cmd; > - memcpy(&op.u, arg, sizeof(op.u)); > - rc = _hypercall1(int, event_channel_op_compat, &op); > - memcpy(arg, &op.u, sizeof(op.u)); > - } > + if (unlikely(rc == -ENOSYS)) > + rc = HYPERVISOR_event_channel_op_compat(cmd, arg); > return rc; > } > > @@ -386,17 +382,14 @@ HYPERVISOR_console_io(int cmd, int count, char *str) > return _hypercall3(int, console_io, cmd, count, str); > } > > +int __must_check HYPERVISOR_physdev_op_compat(int, void *); > + > static inline int > HYPERVISOR_physdev_op(int cmd, void *arg) > { > int rc = _hypercall2(int, physdev_op, cmd, arg); > - if (unlikely(rc == -ENOSYS)) { > - struct physdev_op op; > - op.cmd = cmd; > - memcpy(&op.u, arg, sizeof(op.u)); > - rc = _hypercall1(int, physdev_op_compat, &op); > - memcpy(arg, &op.u, sizeof(op.u)); > - } > + if (unlikely(rc == -ENOSYS)) > + rc = HYPERVISOR_physdev_op_compat(cmd, arg); > return rc; > } > > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > index 0e863703..46de6cd 100644 > --- a/drivers/xen/Makefile > +++ b/drivers/xen/Makefile > @@ -2,7 +2,7 @@ ifneq ($(CONFIG_ARM),y) > obj-y += manage.o balloon.o > obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o > endif > -obj-y += grant-table.o features.o events.o > +obj-y += grant-table.o features.o events.o fallback.o > obj-y += xenbus/ > > nostackp := $(call cc-option, -fno-stack-protector) > diff --git a/drivers/xen/fallback.c b/drivers/xen/fallback.c > new file mode 100644 > index 0000000..0bdc468 > --- /dev/null > +++ b/drivers/xen/fallback.c > @@ -0,0 +1,78 @@ > +#include <linux/kernel.h> > +#include <linux/string.h> > +#include <linux/bug.h> > +#include <asm/hypervisor.h> > +#include <asm/xen/hypercall.h> > + > +int HYPERVISOR_event_channel_op_compat(int cmd, void *arg) > +{ > + struct evtchn_op op; > + int rc; > + > + op.cmd = cmd; > + memcpy(&op.u, arg, sizeof(op.u)); > + rc = _hypercall1(int, event_channel_op_compat, &op); > + > + switch (cmd) { > + case EVTCHNOP_close: > + case EVTCHNOP_send: > + case EVTCHNOP_bind_vcpu: > + case EVTCHNOP_unmask: > + /* no output */ > + break; > + > +#define COPY_BACK(eop) \ > + case EVTCHNOP_##eop: \ > + memcpy(arg, &op.u.eop, sizeof(op.u.eop)); \ > + break > + > + COPY_BACK(bind_interdomain); > + COPY_BACK(bind_virq); > + COPY_BACK(bind_pirq); > + COPY_BACK(status); > + COPY_BACK(alloc_unbound); > + COPY_BACK(bind_ipi); > +#undef COPY_BACK > + > + default: > + WARN_ON(rc != -ENOSYS); > + break; > + } > + > + return rc; > +} > + > +int HYPERVISOR_physdev_op_compat(int cmd, void *arg) > +{ > + struct physdev_op op; > + int rc; > + > + op.cmd = cmd; > + memcpy(&op.u, arg, sizeof(op.u)); > + rc = _hypercall1(int, physdev_op_compat, &op); > + > + switch (cmd) { > + case PHYSDEVOP_IRQ_UNMASK_NOTIFY: > + case PHYSDEVOP_set_iopl: > + case PHYSDEVOP_set_iobitmap: > + case PHYSDEVOP_apic_write: > + /* no output */ > + break; > + > +#define COPY_BACK(pop, fld) \ > + case PHYSDEVOP_##pop: \ > + memcpy(arg, &op.u.fld, sizeof(op.u.fld)); \ > + break > + > + COPY_BACK(irq_status_query, irq_status_query); > + COPY_BACK(apic_read, apic_op); > + COPY_BACK(ASSIGN_VECTOR, irq_op); > +#undef COPY_BACK > + > + default: > + WARN_ON(rc != -ENOSYS); > + break; > + } > + > + return rc; > +} > -- > 1.7.7.6
Jan Beulich
2012-Oct-31 08:55 UTC
Re: [PATCH 1/2] xen/hypercall: fix hypercall fallback code for very old hypervisors
>>> On 30.10.12 at 16:44, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Mon, Oct 29, 2012 at 10:08:17AM -0400, Konrad Rzeszutek Wilk wrote: >> From: Jan Beulich <jbeulich@suse.com> >> >> While copying the argument structures in HYPERVISOR_event_channel_op() >> and HYPERVISOR_physdev_op() into the local variable is sufficiently >> safe even if the actual structure is smaller than the container one, >> copying back eventual output values the same way isn''t: This may >> collide with on-stack variables (particularly "rc") which may change >> between the first and second memcpy() (i.e. the second memcpy() could >> discard that change). >> >> Move the fallback code into out-of-line functions, and handle all of >> the operations known by this old a hypervisor individually: Some don''t >> require copying back anything at all, and for the rest use the >> individual argument structures'' sizes rather than the container''s. >> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> [v2: Reduce #define/#undef usage in HYPERVISOR_physdev_op_compat().] >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > And it looks like I get > > ERROR: "HYPERVISOR_event_channel_op_compat" [drivers/xen/xen-evtchn.ko] > undefined! > > when I build xen-evtchn as module. Jan did you encounter this issue on > 2.6.18?No - the event channel driver there can''t be built as a module, and for the forward ported kernels I apparently never tried to build with a compat setting of 3.0.2 or less (and I didn''t care that much either because the oldest we''re actually concerned about is 3.0.4 to cover some of those very old EC2 systems; I''ll add the export at the right point nevertheless). Jan>> --- >> arch/x86/include/asm/xen/hypercall.h | 21 +++------ >> drivers/xen/Makefile | 2 +- >> drivers/xen/fallback.c | 78 > ++++++++++++++++++++++++++++++++++ >> 3 files changed, 86 insertions(+), 15 deletions(-) >> create mode 100644 drivers/xen/fallback.c >> >> diff --git a/arch/x86/include/asm/xen/hypercall.h > b/arch/x86/include/asm/xen/hypercall.h >> index 59c226d..c812360 100644 >> --- a/arch/x86/include/asm/xen/hypercall.h >> +++ b/arch/x86/include/asm/xen/hypercall.h >> @@ -359,18 +359,14 @@ HYPERVISOR_update_va_mapping(unsigned long va, pte_t > new_val, >> return _hypercall4(int, update_va_mapping, va, >> new_val.pte, new_val.pte >> 32, flags); >> } >> +int __must_check HYPERVISOR_event_channel_op_compat(int, void *); >> >> static inline int >> HYPERVISOR_event_channel_op(int cmd, void *arg) >> { >> int rc = _hypercall2(int, event_channel_op, cmd, arg); >> - if (unlikely(rc == -ENOSYS)) { >> - struct evtchn_op op; >> - op.cmd = cmd; >> - memcpy(&op.u, arg, sizeof(op.u)); >> - rc = _hypercall1(int, event_channel_op_compat, &op); >> - memcpy(arg, &op.u, sizeof(op.u)); >> - } >> + if (unlikely(rc == -ENOSYS)) >> + rc = HYPERVISOR_event_channel_op_compat(cmd, arg); >> return rc; >> } >> >> @@ -386,17 +382,14 @@ HYPERVISOR_console_io(int cmd, int count, char *str) >> return _hypercall3(int, console_io, cmd, count, str); >> } >> >> +int __must_check HYPERVISOR_physdev_op_compat(int, void *); >> + >> static inline int >> HYPERVISOR_physdev_op(int cmd, void *arg) >> { >> int rc = _hypercall2(int, physdev_op, cmd, arg); >> - if (unlikely(rc == -ENOSYS)) { >> - struct physdev_op op; >> - op.cmd = cmd; >> - memcpy(&op.u, arg, sizeof(op.u)); >> - rc = _hypercall1(int, physdev_op_compat, &op); >> - memcpy(arg, &op.u, sizeof(op.u)); >> - } >> + if (unlikely(rc == -ENOSYS)) >> + rc = HYPERVISOR_physdev_op_compat(cmd, arg); >> return rc; >> } >> >> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile >> index 0e863703..46de6cd 100644 >> --- a/drivers/xen/Makefile >> +++ b/drivers/xen/Makefile >> @@ -2,7 +2,7 @@ ifneq ($(CONFIG_ARM),y) >> obj-y += manage.o balloon.o >> obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o >> endif >> -obj-y += grant-table.o features.o events.o >> +obj-y += grant-table.o features.o events.o fallback.o >> obj-y += xenbus/ >> >> nostackp := $(call cc-option, -fno-stack-protector) >> diff --git a/drivers/xen/fallback.c b/drivers/xen/fallback.c >> new file mode 100644 >> index 0000000..0bdc468 >> --- /dev/null >> +++ b/drivers/xen/fallback.c >> @@ -0,0 +1,78 @@ >> +#include <linux/kernel.h> >> +#include <linux/string.h> >> +#include <linux/bug.h> >> +#include <asm/hypervisor.h> >> +#include <asm/xen/hypercall.h> >> + >> +int HYPERVISOR_event_channel_op_compat(int cmd, void *arg) >> +{ >> + struct evtchn_op op; >> + int rc; >> + >> + op.cmd = cmd; >> + memcpy(&op.u, arg, sizeof(op.u)); >> + rc = _hypercall1(int, event_channel_op_compat, &op); >> + >> + switch (cmd) { >> + case EVTCHNOP_close: >> + case EVTCHNOP_send: >> + case EVTCHNOP_bind_vcpu: >> + case EVTCHNOP_unmask: >> + /* no output */ >> + break; >> + >> +#define COPY_BACK(eop) \ >> + case EVTCHNOP_##eop: \ >> + memcpy(arg, &op.u.eop, sizeof(op.u.eop)); \ >> + break >> + >> + COPY_BACK(bind_interdomain); >> + COPY_BACK(bind_virq); >> + COPY_BACK(bind_pirq); >> + COPY_BACK(status); >> + COPY_BACK(alloc_unbound); >> + COPY_BACK(bind_ipi); >> +#undef COPY_BACK >> + >> + default: >> + WARN_ON(rc != -ENOSYS); >> + break; >> + } >> + >> + return rc; >> +} >> + >> +int HYPERVISOR_physdev_op_compat(int cmd, void *arg) >> +{ >> + struct physdev_op op; >> + int rc; >> + >> + op.cmd = cmd; >> + memcpy(&op.u, arg, sizeof(op.u)); >> + rc = _hypercall1(int, physdev_op_compat, &op); >> + >> + switch (cmd) { >> + case PHYSDEVOP_IRQ_UNMASK_NOTIFY: >> + case PHYSDEVOP_set_iopl: >> + case PHYSDEVOP_set_iobitmap: >> + case PHYSDEVOP_apic_write: >> + /* no output */ >> + break; >> + >> +#define COPY_BACK(pop, fld) \ >> + case PHYSDEVOP_##pop: \ >> + memcpy(arg, &op.u.fld, sizeof(op.u.fld)); \ >> + break >> + >> + COPY_BACK(irq_status_query, irq_status_query); >> + COPY_BACK(apic_read, apic_op); >> + COPY_BACK(ASSIGN_VECTOR, irq_op); >> +#undef COPY_BACK >> + >> + default: >> + WARN_ON(rc != -ENOSYS); >> + break; >> + } >> + >> + return rc; >> +} >> -- >> 1.7.7.6
Konrad Rzeszutek Wilk
2012-Nov-02 16:44 UTC
Re: [PATCH 1/2] xen/hypercall: fix hypercall fallback code for very old hypervisors
On Wed, Oct 31, 2012 at 08:55:54AM +0000, Jan Beulich wrote:> >>> On 30.10.12 at 16:44, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > On Mon, Oct 29, 2012 at 10:08:17AM -0400, Konrad Rzeszutek Wilk wrote: > >> From: Jan Beulich <jbeulich@suse.com> > >> > >> While copying the argument structures in HYPERVISOR_event_channel_op() > >> and HYPERVISOR_physdev_op() into the local variable is sufficiently > >> safe even if the actual structure is smaller than the container one, > >> copying back eventual output values the same way isn''t: This may > >> collide with on-stack variables (particularly "rc") which may change > >> between the first and second memcpy() (i.e. the second memcpy() could > >> discard that change). > >> > >> Move the fallback code into out-of-line functions, and handle all of > >> the operations known by this old a hypervisor individually: Some don''t > >> require copying back anything at all, and for the rest use the > >> individual argument structures'' sizes rather than the container''s. > >> > >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >> [v2: Reduce #define/#undef usage in HYPERVISOR_physdev_op_compat().] > >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > And it looks like I get > > > > ERROR: "HYPERVISOR_event_channel_op_compat" [drivers/xen/xen-evtchn.ko] > > undefined! > > > > when I build xen-evtchn as module. Jan did you encounter this issue on > > 2.6.18? > > No - the event channel driver there can''t be built as a module, and > for the forward ported kernels I apparently never tried to build with > a compat setting of 3.0.2 or less (and I didn''t care that much either > because the oldest we''re actually concerned about is 3.0.4 to cover > some of those very old EC2 systems; I''ll add the export at the right > point nevertheless).Ok, ended up with this version which I was thinking to for v3.7-rc5: From 4ae4c7658a7c0501521c422d618038587c3edeca Mon Sep 17 00:00:00 2001 From: Jan Beulich <jbeulich@suse.com> Date: Fri, 19 Oct 2012 15:25:37 -0400 Subject: [PATCH] xen/hypercall: fix hypercall fallback code for very old hypervisors While copying the argument structures in HYPERVISOR_event_channel_op() and HYPERVISOR_physdev_op() into the local variable is sufficiently safe even if the actual structure is smaller than the container one, copying back eventual output values the same way isn''t: This may collide with on-stack variables (particularly "rc") which may change between the first and second memcpy() (i.e. the second memcpy() could discard that change). Move the fallback code into out-of-line functions, and handle all of the operations known by this old a hypervisor individually: Some don''t require copying back anything at all, and for the rest use the individual argument structures'' sizes rather than the container''s. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> [v2: Reduce #define/#undef usage in HYPERVISOR_physdev_op_compat().] [v3: Fix compile errors when modules use said hypercalls] [v4: Add xen_ prefix to the HYPERCALL_..] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/include/asm/xen/hypercall.h | 21 ++++------ drivers/xen/Makefile | 2 +- drivers/xen/fallback.c | 81 ++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 15 deletions(-) create mode 100644 drivers/xen/fallback.c diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h index 59c226d..4055421 100644 --- a/arch/x86/include/asm/xen/hypercall.h +++ b/arch/x86/include/asm/xen/hypercall.h @@ -359,18 +359,14 @@ HYPERVISOR_update_va_mapping(unsigned long va, pte_t new_val, return _hypercall4(int, update_va_mapping, va, new_val.pte, new_val.pte >> 32, flags); } +extern int __must_check xen_HYPERVISOR_event_channel_op_compat(int, void *); static inline int HYPERVISOR_event_channel_op(int cmd, void *arg) { int rc = _hypercall2(int, event_channel_op, cmd, arg); - if (unlikely(rc == -ENOSYS)) { - struct evtchn_op op; - op.cmd = cmd; - memcpy(&op.u, arg, sizeof(op.u)); - rc = _hypercall1(int, event_channel_op_compat, &op); - memcpy(arg, &op.u, sizeof(op.u)); - } + if (unlikely(rc == -ENOSYS)) + rc = xen_HYPERVISOR_event_channel_op_compat(cmd, arg); return rc; } @@ -386,17 +382,14 @@ HYPERVISOR_console_io(int cmd, int count, char *str) return _hypercall3(int, console_io, cmd, count, str); } +extern int __must_check xen_HYPERVISOR_physdev_op_compat(int, void *); + static inline int HYPERVISOR_physdev_op(int cmd, void *arg) { int rc = _hypercall2(int, physdev_op, cmd, arg); - if (unlikely(rc == -ENOSYS)) { - struct physdev_op op; - op.cmd = cmd; - memcpy(&op.u, arg, sizeof(op.u)); - rc = _hypercall1(int, physdev_op_compat, &op); - memcpy(arg, &op.u, sizeof(op.u)); - } + if (unlikely(rc == -ENOSYS)) + rc = xen_HYPERVISOR_physdev_op_compat(cmd, arg); return rc; } diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 0e863703..46de6cd 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,7 +2,7 @@ ifneq ($(CONFIG_ARM),y) obj-y += manage.o balloon.o obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o endif -obj-y += grant-table.o features.o events.o +obj-y += grant-table.o features.o events.o fallback.o obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) diff --git a/drivers/xen/fallback.c b/drivers/xen/fallback.c new file mode 100644 index 0000000..b29ce32 --- /dev/null +++ b/drivers/xen/fallback.c @@ -0,0 +1,81 @@ +#include <linux/kernel.h> +#include <linux/string.h> +#include <linux/bug.h> +#include <linux/export.h> +#include <asm/hypervisor.h> +#include <asm/xen/hypercall.h> + +int xen_HYPERVISOR_event_channel_op_compat(int cmd, void *arg) +{ + struct evtchn_op op; + int rc; + + op.cmd = cmd; + memcpy(&op.u, arg, sizeof(op.u)); + rc = _hypercall1(int, event_channel_op_compat, &op); + + switch (cmd) { + case EVTCHNOP_close: + case EVTCHNOP_send: + case EVTCHNOP_bind_vcpu: + case EVTCHNOP_unmask: + /* no output */ + break; + +#define COPY_BACK(eop) \ + case EVTCHNOP_##eop: \ + memcpy(arg, &op.u.eop, sizeof(op.u.eop)); \ + break + + COPY_BACK(bind_interdomain); + COPY_BACK(bind_virq); + COPY_BACK(bind_pirq); + COPY_BACK(status); + COPY_BACK(alloc_unbound); + COPY_BACK(bind_ipi); +#undef COPY_BACK + + default: + WARN_ON(rc != -ENOSYS); + break; + } + + return rc; +} +EXPORT_SYMBOL_GPL(xen_HYPERVISOR_event_channel_op_compat); + +int xen_HYPERVISOR_physdev_op_compat(int cmd, void *arg) +{ + struct physdev_op op; + int rc; + + op.cmd = cmd; + memcpy(&op.u, arg, sizeof(op.u)); + rc = _hypercall1(int, physdev_op_compat, &op); + + switch (cmd) { + case PHYSDEVOP_IRQ_UNMASK_NOTIFY: + case PHYSDEVOP_set_iopl: + case PHYSDEVOP_set_iobitmap: + case PHYSDEVOP_apic_write: + /* no output */ + break; + +#define COPY_BACK(pop, fld) \ + case PHYSDEVOP_##pop: \ + memcpy(arg, &op.u.fld, sizeof(op.u.fld)); \ + break + + COPY_BACK(irq_status_query, irq_status_query); + COPY_BACK(apic_read, apic_op); + COPY_BACK(ASSIGN_VECTOR, irq_op); +#undef COPY_BACK + + default: + WARN_ON(rc != -ENOSYS); + break; + } + + return rc; +} +EXPORT_SYMBOL_GPL(xen_HYPERVISOR_physdev_op_compat); -- 1.7.11.7
Jan Beulich
2012-Nov-02 16:57 UTC
Re: [PATCH 1/2] xen/hypercall: fix hypercall fallback code for very old hypervisors
>>> On 02.11.12 at 17:44, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote: > --- a/arch/x86/include/asm/xen/hypercall.h > +++ b/arch/x86/include/asm/xen/hypercall.h > @@ -359,18 +359,14 @@ HYPERVISOR_update_va_mapping(unsigned long va, pte_t new_val, > return _hypercall4(int, update_va_mapping, va, > new_val.pte, new_val.pte >> 32, flags); > } > +extern int __must_check xen_HYPERVISOR_event_channel_op_compat(int, void *);Why don''t you drop the HYPERVISOR_ now that you added the xen_?>... > +EXPORT_SYMBOL_GPL(xen_HYPERVISOR_event_channel_op_compat);While this export is necessary, ...>... > +EXPORT_SYMBOL_GPL(xen_HYPERVISOR_physdev_op_compat);... I would recommend not adding this one without need. Jan