From eebc1b2076206600e9ed9ac6b3cd061708f96401 Mon Sep 17 00:00:00 2001

From: Poornima G <pgurusid@redhat.com>

Date: Tue, 14 Feb 2017 12:45:36 +0530

Subject: [PATCH 291/294] rpcsvc: Add rpchdr and proghdr to iobref before

 submitting to transport

Backport of https://review.gluster.org/16613

Issue:

When fio is run on multiple clients (each client writes to its own files),

and meanwhile the clients does a readdirp, thus the client which did

a readdirp will now recieve the upcalls. In this scenario the client

disconnects with rpc decode failed error.

RCA:

Upcall calls rpcsvc_request_submit to submit the request to socket:

rpcsvc_request_submit currently:

rpcsvc_request_submit () {

   iobuf = iobuf_new

   iov = iobuf->ptr

   fill iobuf to contain xdrised upcall content - proghdr

   rpcsvc_callback_submit (..iov..)

   ...

   if (iobuf)

       iobuf_unref (iobuf)

}

rpcsvc_callback_submit (... iov...) {

   ...

   iobuf = iobuf_new

   iov1 = iobuf->ptr

   fill iobuf to contain xdrised rpc header - rpchdr

   msg.rpchdr = iov1

   msg.proghdr = iov

   ...

   rpc_transport_submit_request (msg)

   ...

   if (iobuf)

       iobuf_unref (iobuf)

}

rpcsvc_callback_submit assumes that once rpc_transport_submit_request()

returns the msg is written on to socket and thus the buffers(rpchdr, proghdr)

can be freed, which is not the case. In especially high workload,

rpc_transport_submit_request() may not be able to write to socket immediately

and hence adds it to its own queue and returns as successful. Thus, we have

use after free, for rpchdr and proghdr. Hence the clients gets garbage rpchdr

and proghdr and thus fails to decode the rpc, resulting in disconnect.

To prevent this, we need to add the rpchdr and proghdr to a iobref and send

it in msg:

   iobref_add (iobref, iobufs)

   msg.iobref = iobref;

The socket layer takes a ref on msg.iobref, if it cannot write to socket and

is adding to the queue. Thus we do not have use after free.

Thank You for discussing, debugging and fixing along:

Prashanth Pai <ppai@redhat.com>

Raghavendra G <rgowdapp@redhat.com>

Rajesh Joseph <rjoseph@redhat.com>

Kotresh HR <khiremat@redhat.com>

Mohammed Rafi KC <rkavunga@redhat.com>

Soumya Koduri <skoduri@redhat.com>

> Reviewed-on: https://review.gluster.org/16613

> Reviewed-by: Prashanth Pai <ppai@redhat.com>

> Smoke: Gluster Build System <jenkins@build.gluster.org>

> Reviewed-by: soumya k <skoduri@redhat.com>

> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>

> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>

Change-Id: Ifa6bf6f4879141f42b46830a37c1574b21b37275

BUG: 1409135

Signed-off-by: Poornima G <pgurusid@redhat.com>

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

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

---

 rpc/rpc-lib/src/rpcsvc.c             | 33 +++++++++++++++++++++++++++++++--

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

 xlators/mgmt/glusterd/src/glusterd.c |  6 ++++--

 xlators/protocol/server/src/server.c | 20 +++++++++++++-------

 4 files changed, 50 insertions(+), 12 deletions(-)

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

index 2d2a7ff..641b63c 100644

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

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

@@ -1000,6 +1000,7 @@ int rpcsvc_request_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

         struct iovec            iov         = {0, };

         struct iobuf            *iobuf      = NULL;

         ssize_t                 xdr_size    = 0;

+        struct iobref           *iobref     = NULL;

         if (!req)

                 goto out;

@@ -1022,26 +1023,40 @@ int rpcsvc_request_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

         iov.iov_len = ret;

         count = 1;

+        iobref = iobref_new ();

+        if (!iobref) {

+                ret = -1;

+                gf_log ("rpcsvc", GF_LOG_WARNING, "Failed to create iobref");

+                goto out;

+        }

+

+        iobref_add (iobref, iobuf);

+

         ret = rpcsvc_callback_submit (rpc, trans, prog, procnum,

-                                      &iov, count);

+                                      &iov, count, iobref);

 out:

         if (iobuf)

                 iobuf_unref (iobuf);

+        if (iobref)

+                iobref_unref (iobref);

+

         return ret;

 }

 int

 rpcsvc_callback_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

                         rpcsvc_cbk_program_t *prog, int procnum,

-                        struct iovec *proghdr, int proghdrcount)

+                        struct iovec *proghdr, int proghdrcount,

+                        struct iobref *iobref)

 {

         struct iobuf          *request_iob = NULL;

         struct iovec           rpchdr      = {0,};

         rpc_transport_req_t    req;

         int                    ret         = -1;

         int                    proglen     = 0;

+        gf_boolean_t           new_iobref  = _gf_false;

         if (!rpc) {

                 goto out;

@@ -1063,11 +1078,22 @@ rpcsvc_callback_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

                         "cannot build rpc-record");

                 goto out;

         }

+        if (!iobref) {

+                iobref = iobref_new ();

+                if (!iobref) {

+                        gf_log ("rpcsvc", GF_LOG_WARNING, "Failed to create iobref");

+                        goto out;

+                }

+                new_iobref = 1;

+        }

+

+        iobref_add (iobref, request_iob);

         req.msg.rpchdr = &rpchdr;

         req.msg.rpchdrcount = 1;

         req.msg.proghdr = proghdr;

         req.msg.proghdrcount = proghdrcount;

+        req.msg.iobref = iobref;

         ret = rpc_transport_submit_request (trans, &req;;

         if (ret == -1) {

@@ -1081,6 +1107,9 @@ rpcsvc_callback_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

 out:

         iobuf_unref (request_iob);

+        if (new_iobref)

+               iobref_unref (iobref);

+

         return ret;

 }

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

index 63a6dad..cf3e590 100644

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

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

@@ -581,7 +581,8 @@ int rpcsvc_request_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

 int rpcsvc_callback_submit (rpcsvc_t *rpc, rpc_transport_t *trans,

                             rpcsvc_cbk_program_t *prog, int procnum,

-                            struct iovec *proghdr, int proghdrcount);

+                            struct iovec *proghdr, int proghdrcount,

+                            struct iobref *iobref);

 rpcsvc_actor_t *

 rpcsvc_program_actor (rpcsvc_request_t *req);

diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c

index 498672b..3bcf2d5 100644

--- a/xlators/mgmt/glusterd/src/glusterd.c

+++ b/xlators/mgmt/glusterd/src/glusterd.c

@@ -245,7 +245,8 @@ glusterd_fetchspec_notify (xlator_t *this)

                 list_for_each_entry (trans, &priv->xprt_list, list) {

                         rpcsvc_callback_submit (priv->rpc, trans,

                                                 &glusterd_cbk_prog,

-                                                GF_CBK_FETCHSPEC, NULL, 0);

+                                                GF_CBK_FETCHSPEC, NULL, 0,

+                                                NULL);

                 }

         }

         pthread_mutex_unlock (&priv->xprt_lock);

@@ -281,7 +282,8 @@ glusterd_fetchsnap_notify (xlator_t *this)

                 list_for_each_entry (trans, &priv->xprt_list, list) {

                         rpcsvc_callback_submit (priv->rpc, trans,

                                                 &glusterd_cbk_prog,

-                                                GF_CBK_GET_SNAPS, NULL, 0);

+                                                GF_CBK_GET_SNAPS, NULL, 0,

+                                                NULL);

                 }

         }

         pthread_mutex_unlock (&priv->xprt_lock);

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

index 3c3664d..0bbfd09 100644

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

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

@@ -1289,12 +1289,18 @@ server_process_event_upcall (xlator_t *this, void *data)

                         if (!client || strcmp(client->client_uid, client_uid))

                                 continue;

-                        rpcsvc_request_submit(conf->rpc, xprt,

-                                              &server_cbk_prog,

-                                              cbk_procnum,

-                                              up_req,

-                                              this->ctx,

-                                              xdrproc);

+                        ret = rpcsvc_request_submit (conf->rpc, xprt,

+                                                     &server_cbk_prog,

+                                                     cbk_procnum,

+                                                     up_req,

+                                                     this->ctx,

+                                                     xdrproc);

+                        if (ret < 0) {

+                                gf_msg_debug (this->name, 0, "Failed to send "

+                                              "upcall to client:%s upcall "

+                                              "event:%d", client_uid,

+                                              upcall_data->event_type);

+                        }

                         break;

                 }

         }

@@ -1329,7 +1335,7 @@ server_process_child_event (xlator_t *this, int32_t event, void *data,

                         rpcsvc_callback_submit (conf->rpc, xprt,

                                                 &server_cbk_prog,

                                                 cbk_procnum,

-                                                NULL, 0);

+                                                NULL, 0, NULL);

                 }

         }

         pthread_mutex_unlock (&conf->mutex);

-- 

2.9.3