qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] monitor: properly handle invalid fd/vhostfd fro


From: jason wang
Subject: Re: [Qemu-devel] [PATCH] monitor: properly handle invalid fd/vhostfd from command line
Date: Tue, 12 Oct 2010 14:32:25 +0800
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100915 Thunderbird/3.0.8

On 10/09/2010 01:40 AM, Luiz Capitulino wrote:
On Tue, 28 Sep 2010 16:57:44 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

  
On Tue, Sep 28, 2010 at 11:53:43AM -0300, Luiz Capitulino wrote:
    
On Mon, 27 Sep 2010 15:52:44 +0800
Jason Wang <address@hidden> 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.
    
Sorry for the delay on this one.

If I'm reading this code correctly, we have two kinds of fd passing being
handled by net_handle_fd_param(): fds passed via SCM_RIGHTS (handled by
the monitor) and -net fds, passed via the command-line.

In this case, we don't want the fd passed via SCM_RIGHTS, so
net_handle_fd_param() has to be fixed to check for !mon and also check
strtol()'s return:
  
Yes, it's better to check strtol()'s return which was missed in the original patch.
diff --git a/net.c b/net.c
index 3d0fde7..6aaa653 100644
--- a/net.c
+++ b/net.c
@@ -739,9 +739,9 @@ 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);
@@ -750,7 +750,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;
     }
 }
  
This looks good for me.
There's more: qemu doesn't exit when an invalid fd is passed:

"""
~/stuff/virt/ ./qemu-qmp -hda disks/test.img -enable-kvm -m 1G -snapshot -net tap,fd=40
qemu-qmp: -net tap,fd=40: TUNGETIFF ioctl() failed: Bad file descriptor
TUNSETOFFLOAD ioctl() failed: Bad file descriptor
Warning: vlan 0 with no nics
"""

And QEMU is up and running...
  
Anthony have proposed a patch for this.

  
    
Signed-off-by: Jason Wang <address@hidden>
---
 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;
 


        
    

  


reply via email to

[Prev in Thread] Current Thread [Next in Thread]