Patchwork [RESEND] monitor: properly handle illegal fd/vhostfd from command line

login
register
mail settings
Submitter Jason Wang
Date Oct. 21, 2010, 11:29 a.m.
Message ID <20101021112902.2539.21785.stgit@dhcp-91-7.nay.redhat.com.englab.nay.redhat.com>
Download mbox | patch
Permalink /patch/68592/
State New
Headers show

Comments

Jason Wang - Oct. 21, 2010, 11:29 a.m.
When hanlding fd/vhostfd form command line through net_handle_fd_param(),
we need to check mon and return value of strtol() other than we could
get segmentation fault or invalid fd when user type an illegal fd/vhostfd.

This patch is based on the suggestions from
Luiz Capitulino <lcapitulino@redhat.com>.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
Luiz Capitulino - Oct. 22, 2010, 12:44 p.m.
On Thu, 21 Oct 2010 19:29:02 +0800
Jason Wang <jasowang@redhat.com> wrote:

> When hanlding fd/vhostfd form command line through net_handle_fd_param(),
> we need to check mon and return value of strtol() other than we could
> get segmentation fault or invalid fd when user type an illegal fd/vhostfd.
> 
> This patch is based on the suggestions from
> Luiz Capitulino <lcapitulino@redhat.com>.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  net.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net.c b/net.c
> index ed74c7f..ab9c3bb 100644
> --- a/net.c
> +++ b/net.c
> @@ -774,8 +774,8 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
>  
>  int net_handle_fd_param(Monitor *mon, const char *param)
>  {
> -    if (!qemu_isdigit(param[0])) {
> -        int fd;
> +    int fd;

Better to add a space here.

> +    if (!qemu_isdigit(param[0]) && mon) {
>  
>          fd = monitor_get_fd(mon, param);
>          if (fd == -1) {
> @@ -785,7 +785,13 @@ int net_handle_fd_param(Monitor *mon, const char *param)
>  
>          return fd;
>      } else {
> -        return strtol(param, NULL, 0);
> +        char *endptr = NULL;
> +
> +        fd = strtol(param, &endptr, 10);
> +        if (*endptr || (fd == 0 && param == endptr)) {
> +            return -1;
> +        }
> +        return fd;
>      }

You can put 'return fd' here and drop the two above, also this is not
a monitor patch, it's a networking one.

Otherwise looks ok to me.

>  }
>  
>

Patch

diff --git a/net.c b/net.c
index ed74c7f..ab9c3bb 100644
--- a/net.c
+++ b/net.c
@@ -774,8 +774,8 @@  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
 
 int net_handle_fd_param(Monitor *mon, const char *param)
 {
-    if (!qemu_isdigit(param[0])) {
-        int fd;
+    int fd;
+    if (!qemu_isdigit(param[0]) && mon) {
 
         fd = monitor_get_fd(mon, param);
         if (fd == -1) {
@@ -785,7 +785,13 @@  int net_handle_fd_param(Monitor *mon, const char *param)
 
         return fd;
     } else {
-        return strtol(param, NULL, 0);
+        char *endptr = NULL;
+
+        fd = strtol(param, &endptr, 10);
+        if (*endptr || (fd == 0 && param == endptr)) {
+            return -1;
+        }
+        return fd;
     }
 }