Patchwork [V3] net: properly handle illegal fd/vhostfd from command line

login
register
mail settings
Submitter Jason Wang
Date Oct. 25, 2010, 5:39 a.m.
Message ID <20101025053959.10150.69081.stgit@dhcp-91-7.nay.redhat.com.englab.nay.redhat.com>
Download mbox | patch
Permalink /patch/69066/
State New
Headers show

Comments

Jason Wang - Oct. 25, 2010, 5:39 a.m.
When hanlding fd/vhostfd form command line through net_handle_fd_param(),
we need to check mon and return value of strtol() otherwise 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 |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)
Luiz Capitulino - Oct. 25, 2010, 12:52 p.m.
On Mon, 25 Oct 2010 13:39:59 +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() otherwise 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>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  net.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net.c b/net.c
> index ed74c7f..c5e6063 100644
> --- a/net.c
> +++ b/net.c
> @@ -774,19 +774,25 @@ 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) {
>              error_report("No file descriptor named %s found", param);
>              return -1;
>          }
> -
> -        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;
>  }
>  
>  static int net_init_nic(QemuOpts *opts,
>
Michael S. Tsirkin - Oct. 25, 2010, 4:57 p.m.
On Mon, Oct 25, 2010 at 10:52:05AM -0200, Luiz Capitulino wrote:
> On Mon, 25 Oct 2010 13:39:59 +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() otherwise 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>
> 
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

Applied, thanks everyone.

> > ---
> >  net.c |   16 +++++++++++-----
> >  1 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net.c b/net.c
> > index ed74c7f..c5e6063 100644
> > --- a/net.c
> > +++ b/net.c
> > @@ -774,19 +774,25 @@ 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) {
> >              error_report("No file descriptor named %s found", param);
> >              return -1;
> >          }
> > -
> > -        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;
> >  }
> >  
> >  static int net_init_nic(QemuOpts *opts,
> >
Michael S. Tsirkin - Oct. 26, 2010, 2:58 p.m.
On Mon, Oct 25, 2010 at 01:39:59PM +0800, Jason Wang wrote:
> When hanlding fd/vhostfd form command line through net_handle_fd_param(),
> we need to check mon and return value of strtol() otherwise 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>

Applied.

> ---
>  net.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net.c b/net.c
> index ed74c7f..c5e6063 100644
> --- a/net.c
> +++ b/net.c
> @@ -774,19 +774,25 @@ 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) {
>              error_report("No file descriptor named %s found", param);
>              return -1;
>          }
> -
> -        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;
>  }
>  
>  static int net_init_nic(QemuOpts *opts,

Patch

diff --git a/net.c b/net.c
index ed74c7f..c5e6063 100644
--- a/net.c
+++ b/net.c
@@ -774,19 +774,25 @@  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) {
             error_report("No file descriptor named %s found", param);
             return -1;
         }
-
-        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;
 }
 
 static int net_init_nic(QemuOpts *opts,