Patchwork monitor: properly handle invalid fd/vhostfd from command line

login
register
mail settings
Submitter Jason Wang
Date Sept. 27, 2010, 7:52 a.m.
Message ID <20100927075244.8835.530.stgit@dhcp-91-7.nay.redhat.com.englab.nay.redhat.com>
Download mbox | patch
Permalink /patch/65818/
State New
Headers show

Comments

Jason Wang - Sept. 27, 2010, 7:52 a.m.
monitor_get_fd() may also be used to parse fd or vhostfd from command line, so
we need to check whether the pointer of mon is NULL to avoid segmentation fault
when user pass invalid name of fd or vhostfd.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 monitor.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Michael S. Tsirkin - Sept. 27, 2010, 10:49 a.m.
On Mon, Sep 27, 2010 at 03:52:44PM +0800, Jason Wang wrote:
> monitor_get_fd() may also be used to parse fd or vhostfd from command line, so
> we need to check whether the pointer of mon is NULL to avoid segmentation fault
> when user pass invalid name of fd or vhostfd.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  monitor.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index e602480..5bb4ff0 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2345,6 +2345,10 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
>  {
>      mon_fd_t *monfd;
>  
> +    if (mon == NULL) {
> +        return -1;
> +    }
> +
>      QLIST_FOREACH(monfd, &mon->fds, next) {
>          int fd;
>  
>
Luiz Capitulino - Sept. 28, 2010, 2:53 p.m.
On Mon, 27 Sep 2010 15:52:44 +0800
Jason Wang <jasowang@redhat.com> wrote:

> monitor_get_fd() may also be used to parse fd or vhostfd from command line, so
> we need to check whether the pointer of mon is NULL to avoid segmentation fault
> when user pass invalid name of fd or vhostfd.

Invalid fdname is handled just fine, I have the impression this patch fixes
something else.

Could you elaborate on the real problem here and/or show to reproduce?

> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  monitor.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index e602480..5bb4ff0 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2345,6 +2345,10 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
>  {
>      mon_fd_t *monfd;
>  
> +    if (mon == NULL) {
> +        return -1;
> +    }
> +
>      QLIST_FOREACH(monfd, &mon->fds, next) {
>          int fd;
>  
> 
>
Michael S. Tsirkin - Sept. 28, 2010, 2:57 p.m.
On Tue, Sep 28, 2010 at 11:53:43AM -0300, Luiz Capitulino wrote:
> On Mon, 27 Sep 2010 15:52:44 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
> > monitor_get_fd() may also be used to parse fd or vhostfd from command line, so
> > we need to check whether the pointer of mon is NULL to avoid segmentation fault
> > when user pass invalid name of fd or vhostfd.
> 
> Invalid fdname is handled just fine, I have the impression this patch fixes
> something else.
> 
> Could you elaborate on the real problem here and/or show to reproduce?

Try pasing fd= (no value) as a parameter, and see what happens.


> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  monitor.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/monitor.c b/monitor.c
> > index e602480..5bb4ff0 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2345,6 +2345,10 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
> >  {
> >      mon_fd_t *monfd;
> >  
> > +    if (mon == NULL) {
> > +        return -1;
> > +    }
> > +
> >      QLIST_FOREACH(monfd, &mon->fds, next) {
> >          int fd;
> >  
> > 
> >

Patch

diff --git a/monitor.c b/monitor.c
index e602480..5bb4ff0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2345,6 +2345,10 @@  int monitor_get_fd(Monitor *mon, const char *fdname)
 {
     mon_fd_t *monfd;
 
+    if (mon == NULL) {
+        return -1;
+    }
+
     QLIST_FOREACH(monfd, &mon->fds, next) {
         int fd;