locks: Protected racy (read) access of ext_list

Change-Id: Ibf639695ebd99c11c6960c9be82c0cee71b50744
BUG: 905864
Signed-off-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-on: http://review.gluster.org/4458
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>

3 files changed, 129 insertions, 10 deletions

diff --git a/tests/bugs/bug-905864.c b/tests/bugs/bug-905864.c
new file mode 100644
index 00000000000..ed09b6e2bc4
--- /dev/null
+++ b/tests/bugs/bug-905864.c
@@ -0,0 +1,82 @@
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <pthread.h>
+
+
+pthread_t   th[5] = {0};
+void
+flock_init (struct flock *f, short int type, off_t start, off_t len)
+{
+    f->l_type = type;
+    f->l_start = start;
+    f->l_len = len;
+}
+
+int
+flock_range_in_steps (int fd, int is_set, short l_type,
+                      int start, int end, int step)
+{
+        int         ret = 0;
+        int         i = 0;
+        struct      flock f = {0,};
+
+        for (i = start; i+step < end; i += step) {
+                flock_init (&f, l_type, i, step);
+                ret = fcntl (fd, (is_set)? F_SETLKW:F_GETLK, &f);
+                if (ret) {
+                        perror ("fcntl");
+                        goto out;
+                }
+        }
+out:
+        return ret;
+}
+
+void *
+random_locker (void *arg)
+{
+        int         fd = *(int *)arg;
+        int         i = 0;
+        int         is_set = 0;
+
+        /* use thread id to choose GETLK or SETLK operation*/
+        is_set = pthread_self () % 2;
+        (void)flock_range_in_steps (fd, is_set, F_WRLCK, 0, 400, 1);
+
+        return NULL;
+}
+
+
+int main (int argc, char **argv)
+{
+        int         fd = -1;
+        int         ret = 1;
+        int         i = 0;
+        char        *fname = NULL;
+
+        if (argc < 2)
+                goto out;
+
+        fname = argv[1];
+        fd = open (fname, O_RDWR);
+        if (fd == -1) {
+                perror ("open");
+                goto out;
+        }
+
+        ret = flock_range_in_steps (fd, 1, F_WRLCK, 0, 2000, 2);
+        for (i = 0; i < 5; i++) {
+                pthread_create (&th[i], NULL, random_locker, (void *) &fd);
+        }
+        ret = flock_range_in_steps (fd, 1, F_WRLCK, 0, 2000, 2);
+        for (i = 0; i < 5; i++) {
+                pthread_join (th[i], NULL);
+        }
+out:
+        if (fd != -1)
+                close (fd);
+
+        return ret;
+}
diff --git a/tests/bugs/bug-905864.t b/tests/bugs/bug-905864.t
new file mode 100644
index 00000000000..9d18ea1e9ff
--- /dev/null
+++ b/tests/bugs/bug-905864.t
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+
+cleanup;
+
+## Start and create a volume
+TEST glusterd;
+TEST pidof glusterd;
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 replica 2 stripe 2 $H0:$B0/${V0}{1,2,3,4,5,6,7,8};
+TEST $CLI volume start $V0;
+
+TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0;
+TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M1;
+
+TEST touch $M0/file1;
+
+#following C program tries open up race(s) if any, in F_GETLK/F_SETLKW codepaths
+#of locks xlator
+gcc -lpthread -g3 $(dirname $0)/bug-905864.c -o $(dirname $0)/bug-905864
+$(dirname $0)/bug-905864 $M0/file1 &
+$(dirname $0)/bug-905864 $M1/file1;
+wait
+rm -f $(dirname $0)/bug-905864
+
+cleanup
+
diff --git a/xlators/features/locks/src/common.c b/xlators/features/locks/src/common.c
index cfba16ff735..9c21bddb9e0 100644
--- a/xlators/features/locks/src/common.c
+++ b/xlators/features/locks/src/common.c
@@ -718,22 +718,30 @@ static posix_lock_t *
 first_conflicting_overlap (pl_inode_t *pl_inode, posix_lock_t *lock)
 {
         posix_lock_t *l = NULL;
+        posix_lock_t *conf = NULL;
 
-        list_for_each_entry (l, &pl_inode->ext_list, list) {
-                if (l->blocked)
-                        continue;
-
-                if (locks_overlap (l, lock)) {
-                        if (same_owner (l, lock))
+        pthread_mutex_lock (&pl_inode->mutex);
+        {
+                list_for_each_entry (l, &pl_inode->ext_list, list) {
+                        if (l->blocked)
                                 continue;
 
-                        if ((l->fl_type == F_WRLCK) ||
-                            (lock->fl_type == F_WRLCK))
-                                return l;
+                        if (locks_overlap (l, lock)) {
+                                if (same_owner (l, lock))
+                                        continue;
+
+                                if ((l->fl_type == F_WRLCK) ||
+                                    (lock->fl_type == F_WRLCK)) {
+                                        conf = l;
+                                        goto unlock;
+                                }
+                        }
                 }
         }
+unlock:
+        pthread_mutex_unlock (&pl_inode->mutex);
 
-        return NULL;
+        return conf;
 }
 
 /*