xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Fixes for various minor Coverity issues
@ 2013-09-10 14:34 Matthew Daley
  2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Matthew Daley

These are fixes for various minor non-security-impacting issues discovered
by Coverity. They have been build-tested, but not runtime-tested; they are
hopefully all obvious fixes.

Matthew Daley (8):
  x86: add missing va_end to hypercall_xlat_continuation
  sched/arinc653: check for guest data transfer failures
  libxl: fix use-after-free in discard_events iteration
  libxl: correctly handle readlink() errors
  mini-os: fix use-after-free in xs_daemon_close event iteration
  mini-os: handle possibly overlong _nodename in init_consfront
  kdd: fix free of array-typed value
  xenstored: fix possible, but unlikely, stack overflow

 extras/mini-os/console/xenbus.c  |    6 ++++--
 extras/mini-os/lib/xs.c          |    7 +++++--
 tools/debugger/kdd/kdd-xen.c     |    1 -
 tools/libxl/libxl.c              |    4 ++--
 tools/libxl/libxl_exec.c         |    2 +-
 tools/xenstore/xenstored_linux.c |    2 +-
 xen/arch/x86/domain.c            |    4 ++++
 xen/common/sched_arinc653.c      |   13 +++++++++++--
 8 files changed, 28 insertions(+), 11 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:41   ` Andrew Cooper
  2013-09-10 15:02   ` Keir Fraser
  2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Matthew Daley, Keir Fraser, Jan Beulich

Coverity-ID: 1056208
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 xen/arch/x86/domain.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index f7b0308..316ef04 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1648,7 +1648,11 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
     if ( test_bit(_MCSF_in_multicall, &mcs->flags) )
     {
         if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
+        {
+            va_end(args);
             return 0;
+        }
+
         for ( i = 0; i < 6; ++i, mask >>= 1 )
         {
             if ( mask & 1 )
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
  2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:43   ` Andrew Cooper
                     ` (2 more replies)
  2013-09-10 14:34 ` [PATCH 3/8] libxl: fix use-after-free in discard_events iteration Matthew Daley
                   ` (6 subsequent siblings)
  8 siblings, 3 replies; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Matthew Daley

Coverity-ID: 1055121
Coverity-ID: 1055122
Coverity-ID: 1055123
Coverity-ID: 1055124
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 xen/common/sched_arinc653.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 63ddb82..2502192 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -635,12 +635,21 @@ a653sched_adjust_global(const struct scheduler *ops,
     switch ( sc->cmd )
     {
     case XEN_SYSCTL_SCHEDOP_putinfo:
-        copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1);
+        if ( copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1) )
+        {
+            rc = -EFAULT;
+            break;
+        }
+
         rc = arinc653_sched_set(ops, &local_sched);
         break;
     case XEN_SYSCTL_SCHEDOP_getinfo:
         rc = arinc653_sched_get(ops, &local_sched);
-        copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1);
+        if ( rc )
+            break;
+
+        if ( copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1) )
+            rc = -EFAULT;
         break;
     }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/8] libxl: fix use-after-free in discard_events iteration
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
  2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
  2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:45   ` Ian Jackson
  2013-09-10 14:34 ` [PATCH 4/8] libxl: correctly handle readlink() errors Matthew Daley
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Matthew Daley, Ian Jackson, Ian Campbell, Stefano Stabellini

We need to use the foreach variant which gets the next pointer before
the loop body is executed.

Coverity-ID: 1056193
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 tools/libxl/libxl.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 81785df..8f4a250 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -122,8 +122,8 @@ static void free_disable_deaths(libxl__gc *gc,
 
 static void discard_events(struct libxl__event_list *l) {
     /* doesn't bother unlinking from the list, so l is corrupt on return */
-    libxl_event *ev;
-    LIBXL_TAILQ_FOREACH(ev, l, link)
+    libxl_event *ev, *next;
+    LIBXL_TAILQ_FOREACH_SAFE(ev, l, link, next)
         libxl_event_free(0, ev);
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/8] libxl: correctly handle readlink() errors
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (2 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 3/8] libxl: fix use-after-free in discard_events iteration Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:47   ` Ian Jackson
  2013-09-10 14:34 ` [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration Matthew Daley
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Matthew Daley, Ian Jackson, Ian Campbell, Stefano Stabellini

readlink() returns a ssize_t with a negative value on failure.

Coverity-ID: 1055566
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 tools/libxl/libxl_exec.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 98bfd71..7eddaef 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -33,7 +33,7 @@ static void check_open_fds(const char *what)
 
     for (i = 4; i < 256; i++) {
 #ifdef __linux__
-        size_t len;
+        ssize_t len;
         char path[PATH_MAX];
         char linkpath[PATH_MAX+1];
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (3 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 4/8] libxl: correctly handle readlink() errors Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:37   ` Samuel Thibault
  2013-09-10 14:34 ` [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront Matthew Daley
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Samuel Thibault, Matthew Daley, Stefano Stabellini

We need to get the next pointer before the freeing of the event.

Coverity-ID: 1056173
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 extras/mini-os/lib/xs.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/extras/mini-os/lib/xs.c b/extras/mini-os/lib/xs.c
index a2a1220..c603d17 100644
--- a/extras/mini-os/lib/xs.c
+++ b/extras/mini-os/lib/xs.c
@@ -29,9 +29,12 @@ struct xs_handle *xs_daemon_open()
 void xs_daemon_close(struct xs_handle *h)
 {
     int fd = _xs_fileno(h);
-    struct xenbus_event *event;
-    for (event = files[fd].xenbus.events; event; event = event->next)
+    struct xenbus_event *event, *next;
+    for (event = files[fd].xenbus.events; event; event = next)
+    {
+        next = event->next;
         free(event);
+    }
     files[fd].type = FTYPE_NONE;
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (4 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:38   ` Samuel Thibault
  2013-09-10 14:34 ` [PATCH 7/8] kdd: fix free of array-typed value Matthew Daley
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Samuel Thibault, Matthew Daley, Stefano Stabellini

The only current user that passes a non-NULL _nodename limits it to 64
bytes anyway.

Coverity-ID: 1054993
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 extras/mini-os/console/xenbus.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index e65baf7..95d42a4 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -70,8 +70,10 @@ struct consfront_dev *init_consfront(char *_nodename)
 
     if (!_nodename)
         snprintf(nodename, sizeof(nodename), "device/console/%d", consfrontends);
-    else
-        strncpy(nodename, _nodename, sizeof(nodename));
+    else {
+        strncpy(nodename, _nodename, sizeof(nodename) - 1);
+        nodename[sizeof(nodename) - 1] = 0;
+    }
 
     printk("******************* CONSFRONT for %s **********\n\n\n", nodename);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 7/8] kdd: fix free of array-typed value
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (5 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:41   ` Tim Deegan
  2013-09-10 14:46   ` Andrew Cooper
  2013-09-10 14:34 ` [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow Matthew Daley
  2013-09-13 12:31 ` [PATCH 0/8] Fixes for various minor Coverity issues Ian Campbell
  8 siblings, 2 replies; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Jackson, Matthew Daley, Tim Deegan, Ian Campbell,
	Stefano Stabellini

g->id is an array and is allocated as part of g itself; it's not a
separate allocation.

Coverity-ID: 1054980
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 tools/debugger/kdd/kdd-xen.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index 4fbea7d..f3f9529 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -619,7 +619,6 @@ void kdd_guest_teardown(kdd_guest *g)
 {
     flush_maps(g);
     xc_interface_close(g->xc_handle);
-    free(g->id);
     free(g->hvm_buf);
     free(g);
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (6 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 7/8] kdd: fix free of array-typed value Matthew Daley
@ 2013-09-10 14:34 ` Matthew Daley
  2013-09-10 14:48   ` Andrew Cooper
  2013-09-10 14:48   ` Ian Jackson
  2013-09-13 12:31 ` [PATCH 0/8] Fixes for various minor Coverity issues Ian Campbell
  8 siblings, 2 replies; 25+ messages in thread
From: Matthew Daley @ 2013-09-10 14:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Matthew Daley, Ian Jackson, Ian Campbell, Stefano Stabellini

...when reading xenbus port from xenfs.

Coverity-ID: 1055741
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 tools/xenstore/xenstored_linux.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_linux.c b/tools/xenstore/xenstored_linux.c
index 5460ca5..cf40213 100644
--- a/tools/xenstore/xenstored_linux.c
+++ b/tools/xenstore/xenstored_linux.c
@@ -32,7 +32,7 @@ evtchn_port_t xenbus_evtchn(void)
 	if (fd == -1)
 		return -1;
 
-	rc = read(fd, str, sizeof(str)); 
+	rc = read(fd, str, sizeof(str) - 1);
 	if (rc == -1)
 	{
 		int err = errno;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration
  2013-09-10 14:34 ` [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration Matthew Daley
@ 2013-09-10 14:37   ` Samuel Thibault
  0 siblings, 0 replies; 25+ messages in thread
From: Samuel Thibault @ 2013-09-10 14:37 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, xen-devel

Matthew Daley, le Wed 11 Sep 2013 02:34:19 +1200, a écrit :
> We need to get the next pointer before the freeing of the event.
> 
> Coverity-ID: 1056173
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-By: Samuel Thibault <samuel.thibault@ens-lyon.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront
  2013-09-10 14:34 ` [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront Matthew Daley
@ 2013-09-10 14:38   ` Samuel Thibault
  0 siblings, 0 replies; 25+ messages in thread
From: Samuel Thibault @ 2013-09-10 14:38 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, xen-devel

Matthew Daley, le Wed 11 Sep 2013 02:34:20 +1200, a écrit :
> The only current user that passes a non-NULL _nodename limits it to 64
> bytes anyway.
> 
> Coverity-ID: 1054993
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation
  2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
@ 2013-09-10 14:41   ` Andrew Cooper
  2013-09-10 15:02   ` Keir Fraser
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2013-09-10 14:41 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Keir Fraser, Jan Beulich, xen-devel

On 10/09/13 15:34, Matthew Daley wrote:
> Coverity-ID: 1056208
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/domain.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index f7b0308..316ef04 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1648,7 +1648,11 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
>      if ( test_bit(_MCSF_in_multicall, &mcs->flags) )
>      {
>          if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
> +        {
> +            va_end(args);
>              return 0;
> +        }
> +
>          for ( i = 0; i < 6; ++i, mask >>= 1 )
>          {
>              if ( mask & 1 )

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/8] kdd: fix free of array-typed value
  2013-09-10 14:34 ` [PATCH 7/8] kdd: fix free of array-typed value Matthew Daley
@ 2013-09-10 14:41   ` Tim Deegan
  2013-09-10 14:46   ` Andrew Cooper
  1 sibling, 0 replies; 25+ messages in thread
From: Tim Deegan @ 2013-09-10 14:41 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, Ian Jackson, Ian Campbell, xen-devel

At 02:34 +1200 on 11 Sep (1378866861), Matthew Daley wrote:
> g->id is an array and is allocated as part of g itself; it's not a
> separate allocation.
> 
> Coverity-ID: 1054980
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Tim Deegan <tim@xen.org>

> ---
>  tools/debugger/kdd/kdd-xen.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
> index 4fbea7d..f3f9529 100644
> --- a/tools/debugger/kdd/kdd-xen.c
> +++ b/tools/debugger/kdd/kdd-xen.c
> @@ -619,7 +619,6 @@ void kdd_guest_teardown(kdd_guest *g)
>  {
>      flush_maps(g);
>      xc_interface_close(g->xc_handle);
> -    free(g->id);
>      free(g->hvm_buf);
>      free(g);
>  }
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
@ 2013-09-10 14:43   ` Andrew Cooper
  2013-09-10 14:50   ` George Dunlap
  2013-09-10 15:03   ` Keir Fraser
  2 siblings, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2013-09-10 14:43 UTC (permalink / raw)
  To: Matthew Daley; +Cc: George Dunlap, xen-devel

On 10/09/13 15:34, Matthew Daley wrote:
> Coverity-ID: 1055121
> Coverity-ID: 1055122
> Coverity-ID: 1055123
> Coverity-ID: 1055124
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/common/sched_arinc653.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
> index 63ddb82..2502192 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -635,12 +635,21 @@ a653sched_adjust_global(const struct scheduler *ops,
>      switch ( sc->cmd )
>      {
>      case XEN_SYSCTL_SCHEDOP_putinfo:
> -        copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1);
> +        if ( copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1) )
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
> +
>          rc = arinc653_sched_set(ops, &local_sched);
>          break;
>      case XEN_SYSCTL_SCHEDOP_getinfo:
>          rc = arinc653_sched_get(ops, &local_sched);
> -        copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1);
> +        if ( rc )
> +            break;
> +
> +        if ( copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1) )
> +            rc = -EFAULT;
>          break;
>      }
>  

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/8] libxl: fix use-after-free in discard_events iteration
  2013-09-10 14:34 ` [PATCH 3/8] libxl: fix use-after-free in discard_events iteration Matthew Daley
@ 2013-09-10 14:45   ` Ian Jackson
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2013-09-10 14:45 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, Ian Campbell, xen-devel

Matthew Daley writes ("[PATCH 3/8] libxl: fix use-after-free in discard_events iteration"):
> We need to use the foreach variant which gets the next pointer before
> the loop body is executed.
> 
> Coverity-ID: 1056193
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/8] kdd: fix free of array-typed value
  2013-09-10 14:34 ` [PATCH 7/8] kdd: fix free of array-typed value Matthew Daley
  2013-09-10 14:41   ` Tim Deegan
@ 2013-09-10 14:46   ` Andrew Cooper
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2013-09-10 14:46 UTC (permalink / raw)
  To: Matthew Daley
  Cc: Tim Deegan, Stefano Stabellini, Ian Jackson, Ian Campbell,
	xen-devel

On 10/09/13 15:34, Matthew Daley wrote:
> g->id is an array and is allocated as part of g itself; it's not a
> separate allocation.
>
> Coverity-ID: 1054980
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  tools/debugger/kdd/kdd-xen.c |    1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
> index 4fbea7d..f3f9529 100644
> --- a/tools/debugger/kdd/kdd-xen.c
> +++ b/tools/debugger/kdd/kdd-xen.c
> @@ -619,7 +619,6 @@ void kdd_guest_teardown(kdd_guest *g)
>  {
>      flush_maps(g);
>      xc_interface_close(g->xc_handle);
> -    free(g->id);
>      free(g->hvm_buf);
>      free(g);
>  }

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/8] libxl: correctly handle readlink() errors
  2013-09-10 14:34 ` [PATCH 4/8] libxl: correctly handle readlink() errors Matthew Daley
@ 2013-09-10 14:47   ` Ian Jackson
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2013-09-10 14:47 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, Ian Campbell, xen-devel

Matthew Daley writes ("[PATCH 4/8] libxl: correctly handle readlink() errors"):
> readlink() returns a ssize_t with a negative value on failure.
> 
> Coverity-ID: 1055566
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow
  2013-09-10 14:34 ` [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow Matthew Daley
@ 2013-09-10 14:48   ` Andrew Cooper
  2013-09-10 14:48   ` Ian Jackson
  1 sibling, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2013-09-10 14:48 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, Ian Jackson, Ian Campbell, xen-devel

On 10/09/13 15:34, Matthew Daley wrote:
> ...when reading xenbus port from xenfs.
>
> Coverity-ID: 1055741
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  tools/xenstore/xenstored_linux.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/xenstore/xenstored_linux.c b/tools/xenstore/xenstored_linux.c
> index 5460ca5..cf40213 100644
> --- a/tools/xenstore/xenstored_linux.c
> +++ b/tools/xenstore/xenstored_linux.c
> @@ -32,7 +32,7 @@ evtchn_port_t xenbus_evtchn(void)
>  	if (fd == -1)
>  		return -1;
>  
> -	rc = read(fd, str, sizeof(str)); 
> +	rc = read(fd, str, sizeof(str) - 1);
>  	if (rc == -1)
>  	{
>  		int err = errno;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow
  2013-09-10 14:34 ` [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow Matthew Daley
  2013-09-10 14:48   ` Andrew Cooper
@ 2013-09-10 14:48   ` Ian Jackson
  1 sibling, 0 replies; 25+ messages in thread
From: Ian Jackson @ 2013-09-10 14:48 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Stefano Stabellini, Ian Campbell, xen-devel

Matthew Daley writes ("[PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow"):
> ...when reading xenbus port from xenfs.
> 
> Coverity-ID: 1055741
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
  2013-09-10 14:43   ` Andrew Cooper
@ 2013-09-10 14:50   ` George Dunlap
  2013-09-10 17:35     ` Kathy Hadley
  2013-09-10 15:03   ` Keir Fraser
  2 siblings, 1 reply; 25+ messages in thread
From: George Dunlap @ 2013-09-10 14:50 UTC (permalink / raw)
  To: Matthew Daley; +Cc: Josh Holtrop, Kathy Hadley, xen-devel@lists.xen.org

On Tue, Sep 10, 2013 at 3:34 PM, Matthew Daley <mattjd@gmail.com> wrote:
> Coverity-ID: 1055121
> Coverity-ID: 1055122
> Coverity-ID: 1055123
> Coverity-ID: 1055124
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

CC'ing the authors as well so they can ack / nack / backport as desired.

> ---
>  xen/common/sched_arinc653.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
> index 63ddb82..2502192 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -635,12 +635,21 @@ a653sched_adjust_global(const struct scheduler *ops,
>      switch ( sc->cmd )
>      {
>      case XEN_SYSCTL_SCHEDOP_putinfo:
> -        copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1);
> +        if ( copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1) )
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
> +
>          rc = arinc653_sched_set(ops, &local_sched);
>          break;
>      case XEN_SYSCTL_SCHEDOP_getinfo:
>          rc = arinc653_sched_get(ops, &local_sched);
> -        copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1);
> +        if ( rc )
> +            break;
> +
> +        if ( copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1) )
> +            rc = -EFAULT;
>          break;
>      }
>
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation
  2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
  2013-09-10 14:41   ` Andrew Cooper
@ 2013-09-10 15:02   ` Keir Fraser
  1 sibling, 0 replies; 25+ messages in thread
From: Keir Fraser @ 2013-09-10 15:02 UTC (permalink / raw)
  To: Matthew Daley, xen-devel; +Cc: Jan Beulich

On 10/09/2013 07:34, "Matthew Daley" <mattjd@gmail.com> wrote:

> Coverity-ID: 1056208
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Keir Fraser <keir@xen.org>

> ---
>  xen/arch/x86/domain.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index f7b0308..316ef04 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1648,7 +1648,11 @@ int hypercall_xlat_continuation(unsigned int *id,
> unsigned int mask, ...)
>      if ( test_bit(_MCSF_in_multicall, &mcs->flags) )
>      {
>          if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
> +        {
> +            va_end(args);
>              return 0;
> +        }
> +
>          for ( i = 0; i < 6; ++i, mask >>= 1 )
>          {
>              if ( mask & 1 )

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
  2013-09-10 14:43   ` Andrew Cooper
  2013-09-10 14:50   ` George Dunlap
@ 2013-09-10 15:03   ` Keir Fraser
  2 siblings, 0 replies; 25+ messages in thread
From: Keir Fraser @ 2013-09-10 15:03 UTC (permalink / raw)
  To: Matthew Daley, xen-devel; +Cc: George Dunlap

On 10/09/2013 07:34, "Matthew Daley" <mattjd@gmail.com> wrote:

> Coverity-ID: 1055121
> Coverity-ID: 1055122
> Coverity-ID: 1055123
> Coverity-ID: 1055124
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: Keir Fraser <keir@xen.org>

> ---
>  xen/common/sched_arinc653.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
> index 63ddb82..2502192 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -635,12 +635,21 @@ a653sched_adjust_global(const struct scheduler *ops,
>      switch ( sc->cmd )
>      {
>      case XEN_SYSCTL_SCHEDOP_putinfo:
> -        copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1);
> +        if ( copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1)
> )
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
> +
>          rc = arinc653_sched_set(ops, &local_sched);
>          break;
>      case XEN_SYSCTL_SCHEDOP_getinfo:
>          rc = arinc653_sched_get(ops, &local_sched);
> -        copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1);
> +        if ( rc )
> +            break;
> +
> +        if ( copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1) )
> +            rc = -EFAULT;
>          break;
>      }
>  

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 14:50   ` George Dunlap
@ 2013-09-10 17:35     ` Kathy Hadley
  2013-09-10 19:45       ` Ian Campbell
  0 siblings, 1 reply; 25+ messages in thread
From: Kathy Hadley @ 2013-09-10 17:35 UTC (permalink / raw)
  To: George Dunlap, Matthew Daley
  Cc: Nate Studer, Robert VanVossen, xen-devel@lists.xen.org

Acked-by: Kathy Hadley <kathy.hadley@dornerworks.com>

George,

	This scheduler is now being maintained by Robert VanVossen <robert.vanvossen@dornerworks.com> and Nate Studer <nate.studer@dornerworks.com>.  Is there a way to make sure that future e-mails are sent to them instead of Josh and me?

-----Original Message-----
From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George Dunlap
Sent: Tuesday, September 10, 2013 10:51 AM
To: Matthew Daley
Cc: xen-devel@lists.xen.org; Kathy Hadley; Josh Holtrop
Subject: Re: [Xen-devel] [PATCH 2/8] sched/arinc653: check for guest data transfer failures

On Tue, Sep 10, 2013 at 3:34 PM, Matthew Daley <mattjd@gmail.com> wrote:
> Coverity-ID: 1055121
> Coverity-ID: 1055122
> Coverity-ID: 1055123
> Coverity-ID: 1055124
> Signed-off-by: Matthew Daley <mattjd@gmail.com>

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

CC'ing the authors as well so they can ack / nack / backport as desired.

> ---
>  xen/common/sched_arinc653.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c 
> index 63ddb82..2502192 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -635,12 +635,21 @@ a653sched_adjust_global(const struct scheduler *ops,
>      switch ( sc->cmd )
>      {
>      case XEN_SYSCTL_SCHEDOP_putinfo:
> -        copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1);
> +        if ( copy_from_guest(&local_sched, sc->u.sched_arinc653.schedule, 1) )
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }
> +
>          rc = arinc653_sched_set(ops, &local_sched);
>          break;
>      case XEN_SYSCTL_SCHEDOP_getinfo:
>          rc = arinc653_sched_get(ops, &local_sched);
> -        copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1);
> +        if ( rc )
> +            break;
> +
> +        if ( copy_to_guest(sc->u.sched_arinc653.schedule, &local_sched, 1) )
> +            rc = -EFAULT;
>          break;
>      }
>
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures
  2013-09-10 17:35     ` Kathy Hadley
@ 2013-09-10 19:45       ` Ian Campbell
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2013-09-10 19:45 UTC (permalink / raw)
  To: Kathy Hadley
  Cc: George Dunlap, Matthew Daley, Nate Studer, Robert VanVossen,
	xen-devel@lists.xen.org

On Tue, 2013-09-10 at 17:35 +0000, Kathy Hadley wrote:
> 	This scheduler is now being maintained by Robert VanVossen
> <robert.vanvossen@dornerworks.com> and Nate Studer
> <nate.studer@dornerworks.com>.  Is there a way to make sure that
> future e-mails are sent to them instead of Josh and me?

You (or they) can send a patch against the MAINTAINERS file at the top
level of the Xen source tree.

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/8] Fixes for various minor Coverity issues
  2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
                   ` (7 preceding siblings ...)
  2013-09-10 14:34 ` [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow Matthew Daley
@ 2013-09-13 12:31 ` Ian Campbell
  8 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2013-09-13 12:31 UTC (permalink / raw)
  To: Matthew Daley; +Cc: xen-devel

On Wed, 2013-09-11 at 02:34 +1200, Matthew Daley wrote:

>   libxl: fix use-after-free in discard_events iteration
>   libxl: correctly handle readlink() errors
>   mini-os: fix use-after-free in xs_daemon_close event iteration
>   mini-os: handle possibly overlong _nodename in init_consfront
>   kdd: fix free of array-typed value
>   xenstored: fix possible, but unlikely, stack overflow

I applied these tools/mini-os ones with the relevant acks/reviews given
in the thread.

I assume someone else has/will take care of the Xen ones.

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2013-09-13 12:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-10 14:34 [PATCH 0/8] Fixes for various minor Coverity issues Matthew Daley
2013-09-10 14:34 ` [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation Matthew Daley
2013-09-10 14:41   ` Andrew Cooper
2013-09-10 15:02   ` Keir Fraser
2013-09-10 14:34 ` [PATCH 2/8] sched/arinc653: check for guest data transfer failures Matthew Daley
2013-09-10 14:43   ` Andrew Cooper
2013-09-10 14:50   ` George Dunlap
2013-09-10 17:35     ` Kathy Hadley
2013-09-10 19:45       ` Ian Campbell
2013-09-10 15:03   ` Keir Fraser
2013-09-10 14:34 ` [PATCH 3/8] libxl: fix use-after-free in discard_events iteration Matthew Daley
2013-09-10 14:45   ` Ian Jackson
2013-09-10 14:34 ` [PATCH 4/8] libxl: correctly handle readlink() errors Matthew Daley
2013-09-10 14:47   ` Ian Jackson
2013-09-10 14:34 ` [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration Matthew Daley
2013-09-10 14:37   ` Samuel Thibault
2013-09-10 14:34 ` [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront Matthew Daley
2013-09-10 14:38   ` Samuel Thibault
2013-09-10 14:34 ` [PATCH 7/8] kdd: fix free of array-typed value Matthew Daley
2013-09-10 14:41   ` Tim Deegan
2013-09-10 14:46   ` Andrew Cooper
2013-09-10 14:34 ` [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow Matthew Daley
2013-09-10 14:48   ` Andrew Cooper
2013-09-10 14:48   ` Ian Jackson
2013-09-13 12:31 ` [PATCH 0/8] Fixes for various minor Coverity issues Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).