Merge pull request #2032 from matt335672/chansrv_hang_fix

Chansrv hang fix #2032

common/os_calls.c

@@ -2510,6 +2510,50 @@ g_file_get_size(const char *filename)
 #endif
 }
 
+/*****************************************************************************/
+/* returns device number, -1 on error */
+int
+g_file_get_device_number(const char *filename)
+{
+#if defined(_WIN32)
+    return -1;
+#else
+    struct stat st;
+
+    if (stat(filename, &st) == 0)
+    {
+        return (int)(st.st_dev);
+    }
+    else
+    {
+        return -1;
+    }
+
+#endif
+}
+
+/*****************************************************************************/
+/* returns inode number, -1 on error */
+int
+g_file_get_inode_num(const char *filename)
+{
+#if defined(_WIN32)
+    return -1;
+#else
+    struct stat st;
+
+    if (stat(filename, &st) == 0)
+    {
+        return (int)(st.st_ino);
+    }
+    else
+    {
+        return -1;
+    }
+
+#endif
+}
+
 /*****************************************************************************/
 long
 g_load_library(char *in)

common/os_calls.h

@@ -128,6 +128,8 @@ int      g_create_path(const char *path);
 int      g_remove_dir(const char *dirname);
 int      g_file_delete(const char *filename);
 int      g_file_get_size(const char *filename);
+int      g_file_get_device_number(const char *filename);
+int      g_file_get_inode_num(const char *filename);
 long     g_load_library(char *in);
 int      g_free_library(long lib);
 void    *g_get_proc_address(long lib, const char *name);

common/string_calls.c

@@ -188,7 +188,7 @@ g_strlen(const char *text)
 
 /*****************************************************************************/
 /* locates char in text */
-const char *
+char *
 g_strchr(const char *text, int c)
 {
     if (text == NULL)
@@ -196,12 +196,27 @@ g_strchr(const char *text, int c)
         return 0;
     }
 
-    return strchr(text, c);
+    /* Cast needed to compile with C++ */
+    return (char *)strchr(text, c);
+}
+
+/*****************************************************************************/
+/* locates char in text */
+char *
+g_strrchr(const char *text, int c)
+{
+    if (text == NULL)
+    {
+        return 0;
+    }
+
+    /* Cast needed to compile with C++ */
+    return (char *)strrchr(text, c);
 }
 
 /*****************************************************************************/
 /* locates char in text with length */
-const char *
+char *
 g_strnchr(const char *text, int c, int len)
 {
     if (text == NULL || len <= 0)
@@ -209,7 +224,7 @@ g_strnchr(const char *text, int c, int len)
         return NULL;
     }
 
-    return (const char *)memchr(text, c, len);
+    return (char *)memchr(text, c, len);
 }
 
 /*****************************************************************************/
@@ -680,6 +695,20 @@ g_pos(const char *str, const char *to_find)
     return (pp - str);
 }
 
+/*****************************************************************************/
+
+char *
+g_strstr(const char *haystack, const char *needle)
+{
+    if (haystack == NULL || needle == NULL)
+    {
+        return NULL;
+    }
+
+    /* Cast needed to compile with C++ */
+    return (char *)strstr(haystack, needle);
+}
+
 /*****************************************************************************/
 int
 g_mbstowcs(twchar *dest, const char *src, int n)

common/string_calls.h

@@ -175,8 +175,9 @@ g_bitmask_to_str(int bitmask, const struct bitmask_string[],
                  char delim, char *buff, int bufflen);
 
 int      g_strlen(const char *text);
-const char *g_strchr(const char *text, int c);
-const char *g_strnchr(const char *text, int c, int len);
+char    *g_strchr(const char *text, int c);
+char    *g_strrchr(const char *text, int c);
+char    *g_strnchr(const char *text, int c, int len);
 char    *g_strcpy(char *dest, const char *src);
 char    *g_strncpy(char *dest, const char *src, int len);
 char    *g_strcat(char *dest, const char *src);
@@ -202,6 +203,7 @@ int      g_htoi(char *str);
 int      g_bytes_to_hexstr(const void *bytes, int num_bytes, char *out_str,
                            int bytes_out_str);
 int      g_pos(const char *str, const char *to_find);
+char    *g_strstr(const char *haystack, const char *needle);
 int      g_mbstowcs(twchar *dest, const char *src, int n);
 int      g_wcstombs(char *dest, const twchar *src, int n);
 int      g_strtrim(char *str, int trim_flags);

sesman/chansrv/chansrv_fuse.c

@@ -18,6 +18,11 @@
 
 /* FUSE mount point */
 char g_fuse_root_path[256] = "";
+#ifdef XRDP_FUSE
+static const char *g_fuse_root_path_basename; /* See xfuse_path_in_xfuse_fs() */
+static int g_fuse_root_parent_dev;   /* Ditto */
+static int g_fuse_root_parent_ino;   /* Ditto */
+#endif
 char g_fuse_clipboard_path[256] = ""; /* for clipboard use */
 
 #if defined(HAVE_CONFIG_H)
@@ -122,6 +127,11 @@ void xfuse_devredir_cb_rename_file(struct state_rename *fip,
 void xfuse_devredir_cb_file_close(struct state_close *fip)
 {}
 
+int xfuse_path_in_xfuse_fs(const char *path)
+{
+    return 0;
+}
+
 #else
 
 /******************************************************************************
@@ -324,6 +334,7 @@ static char *g_buffer = 0;
 static int g_fd = 0;
 static tintptr g_bufsize = 0;
 
+
 /* forward declarations for internal access */
 static int xfuse_init_xrdp_fs(void);
 static int xfuse_deinit_xrdp_fs(void);
@@ -461,6 +472,7 @@ int
 xfuse_init(void)
 {
     struct fuse_args args = FUSE_ARGS_INIT(0, NULL);
+    char *p;
 
     /* if already inited, just return */
     if (g_xfuse_inited)
@@ -507,16 +519,59 @@ xfuse_init(void)
         g_snprintf(g_fuse_root_path, sizeof(g_fuse_root_path), "%s/%s",
                    g_getenv("HOME"), g_cfg->fuse_mount_name);
     }
+
+    /* Remove all trailing '/' from the root path */
+    p = g_fuse_root_path + g_strlen(g_fuse_root_path);
+    while ( p > g_fuse_root_path && *(p - 1) == '/')
+    {
+        --p;
+        *p = '\0';
+    }
+
+    /* This shouldn't happen */
+    if (g_strlen(g_fuse_root_path) == 0)
+    {
+        LOG(LOG_LEVEL_ERROR,
+            "Fuse root path is empty after removing trailing '/'");
+        return -1;
+    }
+
     g_snprintf(g_fuse_clipboard_path, 255, "%s/.clipboard", g_fuse_root_path);
 
+    /* Get the characteristics of the parent directory of the FUSE mount
+     * point. Used by xfuse_path_in_xfuse_fs() */
+    p = (char *)g_strrchr(g_fuse_root_path, '/');
+    if (p != NULL)
+    {
+        /* Temporarily finish the root path at this point */
+        *p = '\0';
+        g_fuse_root_path_basename = p + 1;
+        g_fuse_root_parent_dev = g_file_get_device_number(g_fuse_root_path);
+        g_fuse_root_parent_ino = g_file_get_inode_num(g_fuse_root_path);
+        *p = '/';
+    }
+    else
+    {
+        g_fuse_root_parent_dev = -1;
+        g_fuse_root_parent_ino = -1;
+    }
+
+    if (g_fuse_root_parent_dev == -1 || g_fuse_root_parent_ino == -1)
+    {
+        LOG(LOG_LEVEL_ERROR,
+            "Unable to obtain characteristics of directory containing %s",
+            g_fuse_root_path);
+        return -1;
+    }
+
     /* if FUSE mount point does not exist, create it */
     if (!g_directory_exist(g_fuse_root_path))
     {
         (void)g_create_path(g_fuse_root_path);
         if (!g_create_dir(g_fuse_root_path))
         {
-            LOG_DEVEL(LOG_LEVEL_ERROR, "mkdir %s failed. If %s is already mounted, you must "
-                      "first unmount it", g_fuse_root_path, g_fuse_root_path);
+            LOG(LOG_LEVEL_ERROR, "mkdir %s failed. If %s is already mounted, you must "
+                "first unmount it", g_fuse_root_path, g_fuse_root_path);
             return -1;
         }
     }
@@ -1491,6 +1546,77 @@ void xfuse_devredir_cb_file_close(struct state_close *fip)
     free(fip);
 }
 
+/*
+ * Determine is a file is in the FUSE filesystem
+ *
+ * The whole purpose of this function is to avoid deadlocks. If we try
+ * to determine the size (or any other attribute) of a file in the FUSE
+ * filesystem, the kernel will block the calling thread and send a message
+ * to libfuse. Since we're single-threaded, this will result in deadlock.
+ *
+ * Ideally we'd use a stat() of the file to compare with a stat() of
+ * the mountpoint. This won't work however, for the reasons outline above.
+ *
+ * A simple implementation might be to compare the start of the filename
+ * with the FUSE mount point. This will work, but can easily be defeated,
+ * (perhaps unintentionally) by symlinks in the path.
+ *
+ * The approach taken here is to record the basename of the FUSE mount
+ * point directory (e.g. 'thinclient_drives') and look for that in the
+ * passed-in filename as a directory element. If we find it, we look
+ * at the device name and inode number of the parent directory of the
+ * FUSE mount point. If these match we consider the filename to be in
+ * the FUSE filesystem.
+ */
+int xfuse_path_in_xfuse_fs(const char *path)
+{
+    char *wpath = NULL; /* Writeable copy of path */
+    char *p;
+    int blen = g_strlen(g_fuse_root_path_basename);
+    int rv = 0;
+
+    if ((wpath = g_strdup(path)) == NULL)
+    {
+        LOG(LOG_LEVEL_ERROR, "system out of memory");
+    }
+    else
+    {
+        /* Look ahead in the string for the basename of the FUSE mount point */
+        for (p = wpath;
+                (p = g_strstr(p, g_fuse_root_path_basename)) != NULL ;
+                p = p + 1)
+        {
+            /* Is this string preceded by a '/' ? */
+            if (p == wpath || *(p - 1) != '/')
+            {
+                continue;
+            }
+
+            /* Is the string followed by a '/' or '\0' ? We know it's
+             * valid to look ahead this far in the string, as g_strstr()
+             * tells us it is */
+            if (*(p + blen) != '/' && *(p + blen) != '\0')
+            {
+                continue;
+            }
+
+            /* Temporarily terminate the string early, and check the
+             * file system characteristics of the preceding directory */
+            *(p - 1) = '\0';
+
+            if (g_file_get_device_number(wpath) == g_fuse_root_parent_dev &&
+                    g_file_get_inode_num(wpath) == g_fuse_root_parent_ino)
+            {
+                rv = 1;
+                break;
+            }
+            *(p - 1) = '/';
+        }
+        g_free(wpath);
+    }
+    return rv;
+}
+
 /******************************************************************************
 **                                                                           **
 **                           callbacks for fuse                              **

sesman/chansrv/chansrv_fuse.h

@@ -114,4 +114,13 @@ void xfuse_devredir_cb_rename_file(struct state_rename *fip,
 
 void xfuse_devredir_cb_file_close(struct state_close *fip);
 
+/*
+ * Returns true if a filesystem path lies in the FUSE filesystem
+ *
+ * Use to prevent deadlocks. For example, if chansrv tries to open
+ * a file in the FUSE filesystem it will fail, as it will call back
+ * into itself to handle the I/O
+ */
+int xfuse_path_in_xfuse_fs(const char *path);
+
 #endif

sesman/chansrv/clipboard_file.c

@@ -200,17 +200,28 @@ clipboard_get_file(const char *file, int bytes)
     clipboard_check_file(pathname);
     clipboard_check_file(filename);
     g_snprintf(full_fn, 255, "%s/%s", pathname, filename);
+
+    /*
+     * Before we look at the file, see if it's in the FUSE filesystem. If it is,
+     * we can't call normal file checking functions, as these will result in
+     * a deadlock */
+    if (xfuse_path_in_xfuse_fs(full_fn))
+    {
+        LOG(LOG_LEVEL_ERROR, "clipboard_get_file: Can't add client-side file "
+            "%s to clipboard", full_fn);
+        return 1;
+    }
     if (g_directory_exist(full_fn))
     {
-        LOG_DEVEL(LOG_LEVEL_ERROR, "clipboard_get_file: file [%s] is a directory, "
-                  "not supported", full_fn);
+        LOG(LOG_LEVEL_ERROR, "clipboard_get_file: file [%s] is a directory, "
+            "not supported", full_fn);
         flags |= CB_FILE_ATTRIBUTE_DIRECTORY;
         return 1;
     }
     if (!g_file_exist(full_fn))
     {
-        LOG_DEVEL(LOG_LEVEL_ERROR, "clipboard_get_file: file [%s] does not exist",
-                  full_fn);
+        LOG(LOG_LEVEL_ERROR, "clipboard_get_file: file [%s] does not exist",
+            full_fn);
         return 1;
     }
     else