* [PATCH v2] Coverity ID: 1215178
@ 2014-05-22 15:06 Paul Durrant
2014-05-22 15:10 ` Andrew Cooper
0 siblings, 1 reply; 2+ messages in thread
From: Paul Durrant @ 2014-05-22 15:06 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Keir Fraser, Jan Beulich
There are two problems with initializetion of the ioreq_t in hvmemul_do_io():
- vp_eport is uninitialized (because it doesn't need to be) but because the
struct is the subject of a copy in hvm_send_assist_req(), this is flagged
as a problem.
- dir, addr, data_is_ptr, and data may be uninitialized when the struct is
passed to hvmtrace_io_assist(). This is clearly a bug, so the initializ-
ation of at least those fields needs to be moved earlier.
This patch fixes both these problems.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/x86/hvm/emulate.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 904c71a..eac159f 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -53,11 +53,17 @@ static int hvmemul_do_io(
int is_mmio, paddr_t addr, unsigned long *reps, int size,
paddr_t ram_gpa, int dir, int df, void *p_data)
{
- paddr_t value = ram_gpa;
- int value_is_ptr = (p_data == NULL);
struct vcpu *curr = current;
struct hvm_vcpu_io *vio;
- ioreq_t p;
+ ioreq_t p = {
+ .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
+ .addr = addr,
+ .size = size,
+ .dir = dir,
+ .df = df,
+ .data = ram_gpa,
+ .data_is_ptr = (p_data == NULL),
+ };
unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
p2m_type_t p2mt;
struct page_info *ram_page;
@@ -94,15 +100,15 @@ static int hvmemul_do_io(
return X86EMUL_UNHANDLEABLE;
}
- if ( (p_data != NULL) && (dir == IOREQ_WRITE) )
+ if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
{
- memcpy(&value, p_data, size);
+ memcpy(&p.data, p_data, size);
p_data = NULL;
}
vio = &curr->arch.hvm_vcpu.hvm_io;
- if ( is_mmio && !value_is_ptr )
+ if ( is_mmio && !p.data_is_ptr )
{
/* Part of a multi-cycle read or write? */
if ( dir == IOREQ_WRITE )
@@ -146,7 +152,7 @@ static int hvmemul_do_io(
goto finish_access;
case HVMIO_dispatched:
/* May have to wait for previous cycle of a multi-write to complete. */
- if ( is_mmio && !value_is_ptr && (dir == IOREQ_WRITE) &&
+ if ( is_mmio && !p.data_is_ptr && (dir == IOREQ_WRITE) &&
(addr == (vio->mmio_large_write_pa +
vio->mmio_large_write_bytes)) )
{
@@ -179,14 +185,7 @@ static int hvmemul_do_io(
if ( vio->mmio_retrying )
*reps = 1;
- p.dir = dir;
- p.data_is_ptr = value_is_ptr;
- p.type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO;
- p.size = size;
- p.addr = addr;
p.count = *reps;
- p.df = df;
- p.data = value;
if ( dir == IOREQ_WRITE )
hvmtrace_io_assist(is_mmio, &p);
@@ -251,7 +250,7 @@ static int hvmemul_do_io(
if ( p_data != NULL )
memcpy(p_data, &vio->io_data, size);
- if ( is_mmio && !value_is_ptr )
+ if ( is_mmio && !p.data_is_ptr )
{
/* Part of a multi-cycle read or write? */
if ( dir == IOREQ_WRITE )
--
1.7.10.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] Coverity ID: 1215178
2014-05-22 15:06 [PATCH v2] Coverity ID: 1215178 Paul Durrant
@ 2014-05-22 15:10 ` Andrew Cooper
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2014-05-22 15:10 UTC (permalink / raw)
To: Paul Durrant; +Cc: Keir Fraser, Jan Beulich, xen-devel
On 22/05/14 16:06, Paul Durrant wrote:
> There are two problems with initializetion of the ioreq_t in hvmemul_do_io():
>
> - vp_eport is uninitialized (because it doesn't need to be) but because the
> struct is the subject of a copy in hvm_send_assist_req(), this is flagged
> as a problem.
> - dir, addr, data_is_ptr, and data may be uninitialized when the struct is
> passed to hvmtrace_io_assist(). This is clearly a bug, so the initializ-
> ation of at least those fields needs to be moved earlier.
>
> This patch fixes both these problems.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> xen/arch/x86/hvm/emulate.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 904c71a..eac159f 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -53,11 +53,17 @@ static int hvmemul_do_io(
> int is_mmio, paddr_t addr, unsigned long *reps, int size,
> paddr_t ram_gpa, int dir, int df, void *p_data)
> {
> - paddr_t value = ram_gpa;
> - int value_is_ptr = (p_data == NULL);
> struct vcpu *curr = current;
> struct hvm_vcpu_io *vio;
> - ioreq_t p;
> + ioreq_t p = {
> + .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
> + .addr = addr,
> + .size = size,
> + .dir = dir,
> + .df = df,
> + .data = ram_gpa,
> + .data_is_ptr = (p_data == NULL),
> + };
> unsigned long ram_gfn = paddr_to_pfn(ram_gpa);
> p2m_type_t p2mt;
> struct page_info *ram_page;
> @@ -94,15 +100,15 @@ static int hvmemul_do_io(
> return X86EMUL_UNHANDLEABLE;
> }
>
> - if ( (p_data != NULL) && (dir == IOREQ_WRITE) )
> + if ( !p.data_is_ptr && (dir == IOREQ_WRITE) )
> {
> - memcpy(&value, p_data, size);
> + memcpy(&p.data, p_data, size);
> p_data = NULL;
> }
>
> vio = &curr->arch.hvm_vcpu.hvm_io;
>
> - if ( is_mmio && !value_is_ptr )
> + if ( is_mmio && !p.data_is_ptr )
> {
> /* Part of a multi-cycle read or write? */
> if ( dir == IOREQ_WRITE )
> @@ -146,7 +152,7 @@ static int hvmemul_do_io(
> goto finish_access;
> case HVMIO_dispatched:
> /* May have to wait for previous cycle of a multi-write to complete. */
> - if ( is_mmio && !value_is_ptr && (dir == IOREQ_WRITE) &&
> + if ( is_mmio && !p.data_is_ptr && (dir == IOREQ_WRITE) &&
> (addr == (vio->mmio_large_write_pa +
> vio->mmio_large_write_bytes)) )
> {
> @@ -179,14 +185,7 @@ static int hvmemul_do_io(
> if ( vio->mmio_retrying )
> *reps = 1;
>
> - p.dir = dir;
> - p.data_is_ptr = value_is_ptr;
> - p.type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO;
> - p.size = size;
> - p.addr = addr;
> p.count = *reps;
> - p.df = df;
> - p.data = value;
>
> if ( dir == IOREQ_WRITE )
> hvmtrace_io_assist(is_mmio, &p);
> @@ -251,7 +250,7 @@ static int hvmemul_do_io(
> if ( p_data != NULL )
> memcpy(p_data, &vio->io_data, size);
>
> - if ( is_mmio && !value_is_ptr )
> + if ( is_mmio && !p.data_is_ptr )
> {
> /* Part of a multi-cycle read or write? */
> if ( dir == IOREQ_WRITE )
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-05-22 15:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-22 15:06 [PATCH v2] Coverity ID: 1215178 Paul Durrant
2014-05-22 15:10 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.