[PATCH 0/8] Fixes for various minor Coverity issues

[PATCH 0/8] Fixes for various minor Coverity issues

[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

[PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation

[Matthew Daley]

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

Re: [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation

[Andrew Cooper]

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 )

Re: [PATCH 1/8] x86: add missing va_end to hypercall_xlat_continuation

[Keir Fraser]

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 )

[PATCH 2/8] sched/arinc653: check for guest data transfer failures

[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

Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures

[Andrew Cooper]

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;
>      }
>  

Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures

[George Dunlap]

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

Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures

[Kathy Hadley]

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

Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures

[Ian Campbell]

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.

Re: [PATCH 2/8] sched/arinc653: check for guest data transfer failures

[Keir Fraser]

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;
>      }
>  

[PATCH 3/8] libxl: fix use-after-free in discard_events iteration

[Matthew Daley]

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

Re: [PATCH 3/8] libxl: fix use-after-free in discard_events iteration

[Ian Jackson]

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>

[PATCH 4/8] libxl: correctly handle readlink() errors

[Matthew Daley]

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

Re: [PATCH 4/8] libxl: correctly handle readlink() errors

[Ian Jackson]

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>

[PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration

[Matthew Daley]

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

Re: [PATCH 5/8] mini-os: fix use-after-free in xs_daemon_close event iteration

[Samuel Thibault]

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>

[PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront

[Matthew Daley]

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

Re: [PATCH 6/8] mini-os: handle possibly overlong _nodename in init_consfront

[Samuel Thibault]

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>

[PATCH 7/8] kdd: fix free of array-typed value

[Matthew Daley]

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

Re: [PATCH 7/8] kdd: fix free of array-typed value

[Tim Deegan]

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

Re: [PATCH 7/8] kdd: fix free of array-typed value

[Andrew Cooper]

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);
>  }

[PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow

[Matthew Daley]

...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

Re: [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow

[Andrew Cooper]

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;

Re: [PATCH 8/8] xenstored: fix possible, but unlikely, stack overflow

[Ian Jackson]

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>

Re: [PATCH 0/8] Fixes for various minor Coverity issues

[Ian Campbell]

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.