diff options
author | Poornima G <pgurusid@redhat.com> | 2017-02-14 12:45:36 +0530 |
---|---|---|
committer | Raghavendra G <rgowdapp@redhat.com> | 2017-02-15 23:09:36 -0500 |
commit | 8607f22dcd1bc9b84e452ae90102fa9d345ad3db (patch) | |
tree | 23360663fc447f082b843b7d15ab4db27335da91 /xlators/protocol | |
parent | 028626a86ea409f908783b9007c02877f20be43e (diff) |
rpcsvc: Add rpchdr and proghdr to iobref before submitting to transport
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>
Change-Id: Ifa6bf6f4879141f42b46830a37c1574b21b37275
BUG: 1421937
Signed-off-by: Poornima G <pgurusid@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>
Diffstat (limited to 'xlators/protocol')
-rw-r--r-- | xlators/protocol/server/src/server.c | 20 |
1 files changed, 13 insertions, 7 deletions
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c index 35bb80ef1a6..4739c4560a6 100644 --- a/xlators/protocol/server/src/server.c +++ b/xlators/protocol/server/src/server.c @@ -1351,12 +1351,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; } } @@ -1391,7 +1397,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); |