Blob Blame History Raw
From 473a0cb2ac1696c13b1a576b71b26eba941dba77 Mon Sep 17 00:00:00 2001
From: Venky Shankar <vshankar@redhat.com>
Date: Thu, 4 Jun 2015 10:07:38 +0530
Subject: [PATCH 118/129] features/bitrot: fix fd leak in truncate (stub)

The need to perform object versioning in the truncate() code path
required an fd to reuse existing versioning infrastructure that's
used by fd based operations (such as writev(), ftruncate(), etc..).

This tempted the use of anonymous fd which was never ever unref()'d
after use resulting in fd and/or memory leak depending on the code
path taken. Versioning resulted in a dangling file descriptor left
open in the filesystem effecting the signing process of a given
object (no release() would be trigerred, hence no signing would be
performed). On the other hand, cases where the object need not be
versioned, the anonymous fd in still ref()'d resulting in memory
leak (NOTE: there's no "dangling" file descriptor in this case).

BUG: 1227900
Change-Id: Ia62d4eef86d7af3dda19817adb1e461eb6a8886d
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Reviewed-on: http://review.gluster.org/11077
Reviewed-on: https://code.engineering.redhat.com/gerrit/51130
---
 tests/bugs/bitrot/bug-1227996.t                  |   58 ++++++++++++++++++++++
 xlators/features/bit-rot/src/stub/bit-rot-stub.c |   11 +++-
 2 files changed, 66 insertions(+), 3 deletions(-)
 create mode 100644 tests/bugs/bitrot/bug-1227996.t

diff --git a/tests/bugs/bitrot/bug-1227996.t b/tests/bugs/bitrot/bug-1227996.t
new file mode 100644
index 0000000..47ebc42
--- /dev/null
+++ b/tests/bugs/bitrot/bug-1227996.t
@@ -0,0 +1,58 @@
+#!/bin/bash
+
+## Test case for bitrot
+## Tunable object signing waiting time value for bitrot.
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+. $(dirname $0)/../../cluster.rc
+
+SLEEP_TIME=3
+
+cleanup;
+## Start glusterd
+TEST glusterd;
+TEST pidof glusterd;
+
+## Lets create and start the volume
+TEST $CLI volume create $V0 $H0:$B0/${V0}0 $H0:$B0/${V0}1
+TEST $CLI volume start $V0
+
+## Enable bitrot on volume $V0
+TEST $CLI volume bitrot $V0 enable
+
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" get_bitd_count
+
+# wait a bit for oneshot crawler to finish
+sleep $SLEEP_TIME
+
+## Set object expiry time value
+TEST $CLI volume bitrot $V0 signing-time $SLEEP_TIME
+
+## Mount the volume
+TEST $GFS --volfile-server=$H0 --volfile-id=$V0 $M0;
+
+# create and check object signature
+fname="$M0/filezero"
+echo "ZZZ" > $fname
+
+# wait till the object is signed
+sleep `expr $SLEEP_TIME \* 2`
+
+backpath=$(get_backend_paths $fname)
+TEST getfattr -m . -n trusted.bit-rot.signature $backpath
+
+## for now just remove the signature xattr to test for signing
+## upon truncate()
+TEST setfattr -x trusted.bit-rot.signature $backpath
+
+## overwrite the file (truncate(), write())
+echo "XYX" > $fname
+
+# wait till the object is signed
+sleep `expr $SLEEP_TIME \* 2`
+
+# test for new signature
+TEST getfattr -m . -n trusted.bit-rot.signature $backpath
+
+cleanup;
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.c b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
index 69e326b..4f0605d 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
@@ -1434,6 +1434,9 @@ int32_t
 br_stub_truncate_resume (call_frame_t *frame, xlator_t *this, loc_t *loc,
                           off_t offset, dict_t *xdata)
 {
+        br_stub_local_t *local = frame->local;
+
+        fd_unref (local->u.context.fd);
         STACK_WIND (frame, br_stub_ftruncate_cbk, FIRST_CHILD(this),
                     FIRST_CHILD(this)->fops->truncate, loc, offset, xdata);
         return 0;
@@ -1482,14 +1485,14 @@ br_stub_truncate (call_frame_t *frame, xlator_t *this, loc_t *loc,
 
         ret = br_stub_need_versioning (this, fd, &inc_version, &modified, &ctx);
         if (ret)
-                goto unwind;
+                goto cleanup_fd;
 
         if (!inc_version && modified)
                 goto wind;
 
         ret = br_stub_versioning_prep (frame, this, fd, ctx);
         if (ret)
-                goto unwind;
+                goto cleanup_fd;
 
         local = frame->local;
         if (!inc_version) {
@@ -1513,12 +1516,14 @@ br_stub_truncate (call_frame_t *frame, xlator_t *this, loc_t *loc,
  wind:
         STACK_WIND (frame, cbk, FIRST_CHILD(this),
                     FIRST_CHILD(this)->fops->truncate, loc, offset, xdata);
+        fd_unref (fd);
         return 0;
 
  cleanup_local:
         br_stub_cleanup_local (local);
         br_stub_dealloc_local (local);
-
+ cleanup_fd:
+        fd_unref (fd);
  unwind:
         frame->local = NULL;
         STACK_UNWIND_STRICT (truncate, frame, op_ret, op_errno, NULL, NULL,
-- 
1.7.1