From dd56ebdefdc8e3d0fe1bbeea6185b72ba0e57871 Mon Sep 17 00:00:00 2001

From: Krutika Dhananjay <kdhananj@redhat.com>

Date: Mon, 17 Oct 2016 15:13:28 +0530

Subject: [PATCH 141/141] compound fops: Fix file corruption issue

        Backport of: http://review.gluster.org/#/c/15654/

1. Address of a local variable @args is copied into state->req

in server3_3_compound (). But even after the function has gone out of

scope, in server_compound_resume () this pointer is accessed and

dereferenced. This patch fixes that.

2. Compound fops, by virtue of NOT having a vector sizer (like the one

writev has), ends up having both the header and the data (in case one of

its member fops is WRITEV) in the same hdr_iobuf. This buffer was not

being preserved through the lifetime of the compound fop, causing it to

be overwritten by a parallel write fop, even when the writev associated

with the currently executing compound fop is yet to hit the desk, thereby

corrupting the file's data. This is fixed by associating the hdr_iobuf with

the iobref so its memory remains valid through the lifetime of the fop.

3. Also fixed a use-after-free bug in protocol/client in compound fops cbk,

missed by Linux but caught by NetBSD.

Finally, big thanks to Pranith Kumar K and Raghavendra Gowdappa for their

help in debugging this file corruption issue.

Change-Id: I0e926dc84b7cc8b9b34b7433ed392516ecdd44bd

BUG: 1379919

Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/87999

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

Tested-by: Atin Mukherjee <amukherj@redhat.com>

---

 rpc/rpc-lib/src/rpc-transport.c               |    6 +---

 rpc/rpc-lib/src/rpc-transport.h               |    1 -

 rpc/rpc-lib/src/rpcsvc.c                      |    6 ----

 rpc/rpc-lib/src/rpcsvc.h                      |    3 --

 tests/basic/afr/compounded-write-txns.t       |   37 +++++++++++++++++++++++++

 xlators/protocol/client/src/client-rpc-fops.c |    9 +++---

 xlators/protocol/server/src/server-rpc-fops.c |    4 +-

 xlators/protocol/server/src/server.h          |    2 +-

 8 files changed, 46 insertions(+), 22 deletions(-)

 create mode 100644 tests/basic/afr/compounded-write-txns.t

diff --git a/rpc/rpc-lib/src/rpc-transport.c b/rpc/rpc-lib/src/rpc-transport.c

index a74aa22..f3ebe2d 100644

--- a/rpc/rpc-lib/src/rpc-transport.c

+++ b/rpc/rpc-lib/src/rpc-transport.c

@@ -123,10 +123,6 @@ rpc_transport_pollin_destroy (rpc_transport_pollin_t *pollin)

                 iobref_unref (pollin->iobref);

         }

-        if (pollin->hdr_iobuf) {

-                iobuf_unref (pollin->hdr_iobuf);

-        }

-

         if (pollin->private) {

                 /* */

                 GF_FREE (pollin->private);

@@ -158,7 +154,7 @@ rpc_transport_pollin_alloc (rpc_transport_t *this, struct iovec *vector,

         msg->iobref = iobref_ref (iobref);

         msg->private = private;

         if (hdr_iobuf)

-                msg->hdr_iobuf = iobuf_ref (hdr_iobuf);

+                iobref_add (iobref, hdr_iobuf);

 out:

         return msg;

diff --git a/rpc/rpc-lib/src/rpc-transport.h b/rpc/rpc-lib/src/rpc-transport.h

index 227911a..1d4b444 100644

--- a/rpc/rpc-lib/src/rpc-transport.h

+++ b/rpc/rpc-lib/src/rpc-transport.h

@@ -163,7 +163,6 @@ struct rpc_transport_pollin {

         char vectored;

         void *private;

         struct iobref *iobref;

-        struct iobuf  *hdr_iobuf;

         char is_reply;

 };

 typedef struct rpc_transport_pollin rpc_transport_pollin_t;

diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c

index 05d2696..2a2ca64 100644

--- a/rpc/rpc-lib/src/rpcsvc.c

+++ b/rpc/rpc-lib/src/rpcsvc.c

@@ -373,9 +373,6 @@ rpcsvc_request_destroy (rpcsvc_request_t *req)

                 iobref_unref (req->iobref);

         }

-        if (req->hdr_iobuf)

-                iobuf_unref (req->hdr_iobuf);

-

         /* This marks the "end" of an RPC request. Reply is

            completely written to the socket and is on the way

            to the client. It is time to decrement the

@@ -690,9 +687,6 @@ rpcsvc_handle_rpc_call (rpcsvc_t *svc, rpc_transport_t *trans,

                 }

                 if (req->synctask) {

-                        if (msg->hdr_iobuf)

-                                req->hdr_iobuf = iobuf_ref (msg->hdr_iobuf);

-

                         ret = synctask_new (THIS->ctx->env,

                                             (synctask_fn_t) actor_fn,

                                             rpcsvc_check_and_reply_error, NULL,

diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h

index 02e467e..63a6dad 100644

--- a/rpc/rpc-lib/src/rpcsvc.h

+++ b/rpc/rpc-lib/src/rpcsvc.h

@@ -244,9 +244,6 @@ struct rpcsvc_request {

         /* Container for transport to store request-specific item */

         void                    *trans_private;

-        /* we need to ref the 'iobuf' in case of 'synctasking' it */

-        struct iobuf            *hdr_iobuf;

-

         /* pointer to cached reply for use in DRC */

         drc_cached_op_t         *reply;

 };

diff --git a/tests/basic/afr/compounded-write-txns.t b/tests/basic/afr/compounded-write-txns.t

new file mode 100644

index 0000000..7cecd87

--- /dev/null

+++ b/tests/basic/afr/compounded-write-txns.t

@@ -0,0 +1,37 @@

+#!/bin/bash

+. $(dirname $0)/../../include.rc

+. $(dirname $0)/../../volume.rc

+

+cleanup

+

+TEST glusterd

+TEST pidof glusterd

+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2}

+TEST $CLI volume set $V0 write-behind off

+TEST $CLI volume set $V0 client-io-threads off

+TEST $CLI volume start $V0

+TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0

+

+# Create and generate data into a src file

+

+TEST `printf %1024s |tr " " "1" > /tmp/source`

+TEST `printf %1024s |tr " " "2" >> /tmp/source`

+

+TEST dd if=/tmp/source of=$M0/file bs=1024 count=2 2>/dev/null

+md5sum_file=$(md5sum $M0/file | awk '{print $1}')

+

+TEST $CLI volume set $V0 cluster.use-compound-fops on

+

+TEST dd if=$M0/file of=$M0/file-copy bs=1024 count=2 2>/dev/null

+

+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0

+TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0

+

+EXPECT "$md5sum_file" echo `md5sum $M0/file-copy | awk '{print $1}'`

+

+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0

+TEST $CLI volume stop $V0

+TEST $CLI volume delete $V0

+

+TEST rm -f /tmp/source

+cleanup

diff --git a/xlators/protocol/client/src/client-rpc-fops.c b/xlators/protocol/client/src/client-rpc-fops.c

index c122941..8085aec 100644

--- a/xlators/protocol/client/src/client-rpc-fops.c

+++ b/xlators/protocol/client/src/client-rpc-fops.c

@@ -3166,7 +3166,8 @@ client3_3_compound_cbk (struct rpc_req *req, struct iovec *iov, int count,

         dict_t                  *xdata           = NULL;

         clnt_local_t            *local           = NULL;

         int                     op_errno         = 0;

-        int                     i,length         = 0;

+        int                     i                = 0;

+        int                     length           = 0;

         int                     ret              = -1;

         this = THIS;

@@ -3187,12 +3188,12 @@ client3_3_compound_cbk (struct rpc_req *req, struct iovec *iov, int count,

                 goto out;

         }

+        length =  local->length;

+

         GF_PROTOCOL_DICT_UNSERIALIZE (this, xdata, (rsp.xdata.xdata_val),

                                       (rsp.xdata.xdata_len), rsp.op_ret,

                                        rsp.op_errno, out);

-        length =  local->length;

-

         args_cbk = compound_args_cbk_alloc (length, xdata);

         if (!args_cbk) {

                 op_errno = ENOMEM;

@@ -3215,7 +3216,7 @@ out:

         free (rsp.xdata.xdata_val);

-        client_compound_rsp_cleanup (&rsp, local->length);

+        client_compound_rsp_cleanup (&rsp, length);

         if (xdata)

                 dict_unref (xdata);

diff --git a/xlators/protocol/server/src/server-rpc-fops.c b/xlators/protocol/server/src/server-rpc-fops.c

index 756275e..d8ad6a7 100644

--- a/xlators/protocol/server/src/server-rpc-fops.c

+++ b/xlators/protocol/server/src/server-rpc-fops.c

@@ -3332,7 +3332,7 @@ server_compound_resume (call_frame_t *frame, xlator_t *bound_xl)

                 goto err;

         }

-        req = state->req;

+        req = &state->req;

         length = req->compound_req_array.compound_req_array_len;

         state->args = compound_fop_alloc (length, req->compound_fop_enum,

@@ -6736,7 +6736,7 @@ server3_3_compound (rpcsvc_request_t *req)

                 goto out;

         }

-        state->req           = &arg;;

+        state->req           = args;

         state->iobref        = iobref_ref (req->iobref);

         if (len < req->msg[0].iov_len) {

diff --git a/xlators/protocol/server/src/server.h b/xlators/protocol/server/src/server.h

index f50e9ab..3272106 100644

--- a/xlators/protocol/server/src/server.h

+++ b/xlators/protocol/server/src/server.h

@@ -191,7 +191,7 @@ struct _server_state {

         struct gf_lease   lease;

         lock_migration_info_t locklist;

         /* required for compound fops */

-        gfs3_compound_req *req;

+        gfs3_compound_req  req;

         /* last length till which iovec for compound

          * writes was processed */

         int               write_length;

-- 

1.7.1