[v11,1/2] libxl: add support for vscsi

Commit Message

Olaf Hering April 8, 2016, 3:43 p.m. UTC

On Fri, Apr 08, Wei Liu wrote:

> On Fri, Apr 08, 2016 at 04:16:46PM +0200, Olaf Hering wrote:
> > What do you have in mind? Something like in vusb_be_from_xs_fe?
> Yes.

Thanks, I will test this change:


    Sanitize input from frontend in vscsi_fill_ctrl

    Signed-off-by: Olaf Hering <[e-mail]>

Comments

Olaf Hering April 11, 2016, 12:57 p.m. UTC | #1

On Fri, Apr 08, Olaf Hering wrote:

> +    r = sscanf(be_path,"/local/domain/%d/backend/%15[^/]/%d",
> +               &be_domid, be_type, &fe_domid);

Why are these values parsed as signed integers, and then placed in
uint32? Was this just an oversight during review?

Olaf

Wei Liu April 11, 2016, 1:02 p.m. UTC | #2

On Mon, Apr 11, 2016 at 02:57:20PM +0200, Olaf Hering wrote:
> On Fri, Apr 08, Olaf Hering wrote:
> 
> > +    r = sscanf(be_path,"/local/domain/%d/backend/%15[^/]/%d",
> > +               &be_domid, be_type, &fe_domid);
> 
> Why are these values parsed as signed integers, and then placed in
> uint32? Was this just an oversight during review?
> 

I think this is oversight.

Wei.

> Olaf

Patch

diff --git a/tools/libxl/libxl_vscsi.c b/tools/libxl/libxl_vscsi.c
index cbaba82..413e129 100644
--- a/tools/libxl/libxl_vscsi.c
+++ b/tools/libxl/libxl_vscsi.c
@@ -117,27 +117,34 @@  static bool vscsi_fill_dev(libxl__gc *gc,
 }
 
 static bool vscsi_fill_ctrl(libxl__gc *gc,
+                            uint32_t tgt_domid,
                             xs_transaction_t t,
                             const char *fe_path,
                             const char *dir,
                             libxl_device_vscsictrl *ctrl)
 {
     libxl_device_vscsidev dev;
-    char *tmp, *be_path, *devs_path;
+    char *tmp, *devs_path;
+    const char *be_path;
     char **dev_dirs;
     unsigned int ndev_dirs, dev_dir;
+    uint32_t be_domid, fe_domid;
+    char be_type[16];
+    int r;
     bool ok;
 
     ctrl->devid = atoi(dir);
 
-    be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, dir));
-    if (!be_path)
+    tmp = GCSPRINTF("%s/%s/backend", fe_path, dir);
+    r = libxl__xs_read_checked(gc, t, tmp, &be_path);
+    if (r || !be_path)
         goto out;
 
-    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, dir));
-    if (!tmp)
+    r = sscanf(be_path,"/local/domain/%d/backend/%15[^/]/%d",
+               &be_domid, be_type, &fe_domid);
+    if (r != 3 || fe_domid != tgt_domid)
         goto out;
-    ctrl->backend_domid = atoi(tmp);
+    ctrl->backend_domid = be_domid;
 
     tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/idx", be_path));
     if (!tmp)
@@ -205,7 +212,7 @@  static int vscsi_collect_ctrls(libxl__gc *gc,
             libxl_device_vscsictrl_init(*ctrls + dir);
 
             libxl_device_vscsictrl_init(&ctrl);
-            if (vscsi_fill_ctrl(gc, t, fe_path, dirs[dir], &ctrl)) {
+            if (vscsi_fill_ctrl(gc, domid, t, fe_path, dirs[dir], &ctrl)) {
                 libxl_device_vscsictrl_copy(CTX, *ctrls + *num, &ctrl);
                 (*num)++;
             }