fuse: use-after-free fix in fuse-bridge, revisited

Prompted by the email exchange in gluster-devel between Oleksandr
Natalenko, xavi, and soumyak, I looked at this because the fuse client
on the longevity cluster has also been suffering from a serious memory
leak for some time. (longevity cluster is currently running 3.7.6)

The longevity cluster manifests the same kernel notifier loop terminated
log message the Oleksandr sees, and some sample runs suggest that the
length passed to the (sys_)write call is unexpectedly and abnormally large.

Basically this fix
 a) uses correct types for len and rv,
 b) copies the len from potentially incorrectly aligned memory (in a
    way that should minimize potential performance issues related to
    accessing unaligned memory.)
 c) changes log level of the kernel notifier loop terminated message
 d) fixes a potential mutex lock/unlock issue

See
    Change-Id: Icedb3525706f59803878bb37ef6b4ffe4a986880
    BUG: 1288857
    http://review.gluster.org/13274

Change-Id: I1a2f957a95978468baa33c2b1f3667934d88939c
BUG: 1288922
Signed-off-by: Kaleb S KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.org/13275
Smoke: Gluster Build System <jenkins@build.gluster.com>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>

1 files changed, 37 insertions, 13 deletions

diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 72dd60c4ed4..bd7d5adf9db 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -3836,12 +3836,13 @@ fuse_setlk (xlator_t *this, fuse_in_header_t *finh, void *msg)
 static void *
 notify_kernel_loop (void *data)
 {
+        uint32_t                 len = 0;
+        ssize_t                   rv = 0;
         xlator_t               *this = NULL;
         fuse_private_t         *priv = NULL;
-        struct fuse_out_header *fouh = NULL;
-        ssize_t                 rv   = 0;
-        ssize_t                 len  = 0;
         fuse_invalidate_node_t *node = NULL;
+        fuse_invalidate_node_t  *tmp = NULL;
+        struct fuse_out_header *pfoh = NULL;
 
         this = data;
         priv = this->private;
@@ -3856,28 +3857,51 @@ notify_kernel_loop (void *data)
                         node = list_entry (priv->invalidate_list.next,
                                            fuse_invalidate_node_t, next);
 
-                        if (node == NULL)
-                                continue;
-
                         list_del_init (&node->next);
                 }
                 pthread_mutex_unlock (&priv->invalidate_mutex);
 
+                pfoh = (struct fuse_out_header *)node->inval_buf;
+                memcpy (&len, &pfoh->len, sizeof(len));
+                /*
+                 * a simple
+                 *         len = pfoh->len;
+                 * works on x86, but takes a multiple insn cycle hit
+                 * when pfoh->len is not correctly aligned, possibly
+                 * even stalling the insn pipeline.
+                 * Other architectures will not be so forgiving. If
+                 * we're lucky the memcpy will be inlined by the
+                 * compiler, and might be as fast or faster without
+                 * the risk of stalling the insn pipeline.
+                 */
 
-                fouh = (struct fuse_out_header *)node->inval_buf;
+                rv = write (priv->fd, node->inval_buf, len);
 
-                len = fouh->len;
-                rv = write (priv->fd, node->inval_buf, fouh->len);
+                GF_FREE (node);
 
-                if (rv != len && !(rv == -1 && errno == ENOENT))
+                if (rv == -1 && errno == EBADF)
                         break;
-                GF_FREE (node);
+
+                if (rv != len && !(rv == -1 && errno == ENOENT)) {
+                        gf_log ("glusterfs-fuse", GF_LOG_INFO,
+                                "len: %u, rv: %zd, errno: %d", len, rv, errno);
+                }
         }
 
-        gf_log ("glusterfs-fuse", GF_LOG_INFO,
+        gf_log ("glusterfs-fuse", GF_LOG_ERROR,
                 "kernel notifier loop terminated");
 
-        GF_FREE (node);
+        pthread_mutex_lock (&priv->invalidate_mutex);
+        {
+                priv->reverse_fuse_thread_started = _gf_false;
+                list_for_each_entry_safe (node, tmp, &priv->invalidate_list,
+                                          next) {
+                        list_del_init (&node->next);
+                        GF_FREE (node);
+                }
+        }
+        pthread_mutex_unlock (&priv->invalidate_mutex);
+
         return NULL;
 }
 #endif