From b6b943038a23c5fcae457d179f84fc8f1d79d8a5 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sat, 16 Aug 2014 16:31:32 +0200 Subject: [PATCH 1/3] core/rpc: avoid double close Reported by coverity as CID #1231253 --- core/rpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/rpc.c b/core/rpc.c index 26e1773c..5aab1503 100644 --- a/core/rpc.c +++ b/core/rpc.c @@ -198,12 +198,13 @@ char *uwsgi_do_rpc(char *node, char *func, uint8_t argc, char *argv[], uint16_t close(fd); *len = rlen; if (*len == 0) { - goto error; + goto error2; } return buffer; error: close(fd); +error2: free(buffer); return NULL; From b94727ef26872ea27b874c1fa7d718e9da1be30e Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sat, 16 Aug 2014 16:45:56 +0200 Subject: [PATCH 2/3] plugins/rpc: fix a couple of memory leaks Always free response, in the worst case we are freeing NULL which is a NOP. Reported by Coverity as CID #1231246, #1231245 --- plugins/rpc/rpc_plugin.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/rpc/rpc_plugin.c b/plugins/rpc/rpc_plugin.c index 134d8864..8724a5ff 100644 --- a/plugins/rpc/rpc_plugin.c +++ b/plugins/rpc/rpc_plugin.c @@ -387,6 +387,7 @@ end: } static int uwsgi_routing_func_rpc_raw(struct wsgi_request *wsgi_req, struct uwsgi_route *ur) { + char *response = NULL; int ret = -1; // this is the list of args char *argv[UMAX8]; @@ -418,7 +419,7 @@ static int uwsgi_routing_func_rpc_raw(struct wsgi_request *wsgi_req, struct uwsg remote = at+1; } uint64_t size; - char *response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); + response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); if (!response) goto end; @@ -426,12 +427,13 @@ static int uwsgi_routing_func_rpc_raw(struct wsgi_request *wsgi_req, struct uwsg if (size == 0) goto end; ret = uwsgi_blob_to_response(wsgi_req, response, size); - free(response); if (ret == 0) { ret = UWSGI_ROUTE_BREAK; } end: + free(response); + for(i=0;icustom;i++) { if (ubs[i] != NULL) { uwsgi_buffer_destroy(ubs[i]); @@ -441,6 +443,7 @@ end: } static int uwsgi_routing_func_rpc_var(struct wsgi_request *wsgi_req, struct uwsgi_route *ur) { + char *response = NULL; int ret = -1; // this is the list of args char *argv[UMAX8]; @@ -472,7 +475,7 @@ static int uwsgi_routing_func_rpc_var(struct wsgi_request *wsgi_req, struct uwsg remote = at+1; } uint64_t size; - char *response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); + response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); if (!response) goto end; @@ -483,9 +486,9 @@ static int uwsgi_routing_func_rpc_var(struct wsgi_request *wsgi_req, struct uwsg free(response); goto end; } - free(response); ret = UWSGI_ROUTE_NEXT; end: + free(response); for(i=0;icustom;i++) { if (ubs[i] != NULL) { uwsgi_buffer_destroy(ubs[i]); From 89bdb35ebc238342e936dd8c5fa5b30ba85231f3 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sat, 16 Aug 2014 17:34:27 +0200 Subject: [PATCH 3/3] plugins/rpc: cleanup duplicated code Introduce a couple of helpers to reduce duplicated code. Before: text data bss dec hex filename 9742 180 0 9922 26c2 plugins/rpc/rpc_plugin.o After: text data bss dec hex filename 9230 180 0 9410 24c2 plugins/rpc/rpc_plugin.o My compiler inlines so uwsgi_rpc_get_remote it does make difference in practice still it makes the code prettier. --- plugins/rpc/rpc_plugin.c | 137 ++++++++++++++------------------------- 1 file changed, 47 insertions(+), 90 deletions(-) diff --git a/plugins/rpc/rpc_plugin.c b/plugins/rpc/rpc_plugin.c index 8724a5ff..ed8a0f2d 100644 --- a/plugins/rpc/rpc_plugin.c +++ b/plugins/rpc/rpc_plugin.c @@ -279,6 +279,38 @@ sendbody: } #ifdef UWSGI_ROUTING +static int uwsgi_rpc_apply_translations(struct wsgi_request *wsgi_req, struct uwsgi_route *ur, struct uwsgi_buffer **ubs, uint64_t *ubs_len, char **argv, uint16_t *argvs) { + uint64_t i; + char **r_argv = (char **) ur->data2; + uint16_t *r_argvs = (uint16_t *) ur->data3; + + char **subject = (char **) (((char *)(wsgi_req))+ur->subject); + uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); + + for(i=0;icustom;i++) { + ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); + if (!ubs[i]) { + *ubs_len = i; + return 0; + } + argv[i] = ubs[i]->buf; + argvs[i] = ubs[i]->pos; + } + + *ubs_len = i; + return 1; +} + +static char *uwsgi_rpc_get_remote(char *func) { + char *remote = NULL; + char *at = strchr(func, '@'); + if (at) { + *at = 0; + remote = at+1; + } + return remote; +} + static int uwsgi_routing_func_rpc(struct wsgi_request *wsgi_req, struct uwsgi_route *ur) { int ret = -1; // this is the list of args @@ -288,28 +320,13 @@ static int uwsgi_routing_func_rpc(struct wsgi_request *wsgi_req, struct uwsgi_ro // this is a placeholder for tmp uwsgi_buffers struct uwsgi_buffer *ubs[UMAX8]; - char **r_argv = (char **) ur->data2; - uint16_t *r_argvs = (uint16_t *) ur->data3; - - char **subject = (char **) (((char *)(wsgi_req))+ur->subject); - uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); - uint64_t i; - for(i=0;icustom;i++) { - ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); - if (!ubs[i]) goto end; - argv[i] = ubs[i]->buf; - argvs[i] = ubs[i]->pos; - } + if (!uwsgi_rpc_apply_translations(wsgi_req, ur, ubs, &i, argv, argvs)) + goto end; // ok we now need to check it it is a local call or a remote one char *func = uwsgi_str(ur->data); - char *remote = NULL; - char *at = strchr(func, '@'); - if (at) { - *at = 0; - remote = at+1; - } + char *remote = uwsgi_rpc_get_remote(func); uint64_t size; char *response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); @@ -340,28 +357,13 @@ static int uwsgi_routing_func_rpc_blob(struct wsgi_request *wsgi_req, struct uws // this is a placeholder for tmp uwsgi_buffers struct uwsgi_buffer *ubs[UMAX8]; - char **r_argv = (char **) ur->data2; - uint16_t *r_argvs = (uint16_t *) ur->data3; - - char **subject = (char **) (((char *)(wsgi_req))+ur->subject); - uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); - uint64_t i; - for(i=0;icustom;i++) { - ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); - if (!ubs[i]) goto end; - argv[i] = ubs[i]->buf; - argvs[i] = ubs[i]->pos; - } + if (!uwsgi_rpc_apply_translations(wsgi_req, ur, ubs, &i, argv, argvs)) + goto end; // ok we now need to check it it is a local call or a remote one char *func = uwsgi_str(ur->data); - char *remote = NULL; - char *at = strchr(func, '@'); - if (at) { - *at = 0; - remote = at+1; - } + char *remote = uwsgi_rpc_get_remote(func); uint64_t size; char *response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); @@ -396,28 +398,13 @@ static int uwsgi_routing_func_rpc_raw(struct wsgi_request *wsgi_req, struct uwsg // this is a placeholder for tmp uwsgi_buffers struct uwsgi_buffer *ubs[UMAX8]; - char **r_argv = (char **) ur->data2; - uint16_t *r_argvs = (uint16_t *) ur->data3; - - char **subject = (char **) (((char *)(wsgi_req))+ur->subject); - uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); - uint64_t i; - for(i=0;icustom;i++) { - ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); - if (!ubs[i]) goto end; - argv[i] = ubs[i]->buf; - argvs[i] = ubs[i]->pos; - } + if (!uwsgi_rpc_apply_translations(wsgi_req, ur, ubs, &i, argv, argvs)) + goto end; // ok we now need to check it it is a local call or a remote one char *func = uwsgi_str(ur->data); - char *remote = NULL; - char *at = strchr(func, '@'); - if (at) { - *at = 0; - remote = at+1; - } + char *remote = uwsgi_rpc_get_remote(func); uint64_t size; response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); @@ -452,28 +439,13 @@ static int uwsgi_routing_func_rpc_var(struct wsgi_request *wsgi_req, struct uwsg // this is a placeholder for tmp uwsgi_buffers struct uwsgi_buffer *ubs[UMAX8]; - char **r_argv = (char **) ur->data2; - uint16_t *r_argvs = (uint16_t *) ur->data3; - - char **subject = (char **) (((char *)(wsgi_req))+ur->subject); - uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); - uint64_t i; - for(i=0;icustom;i++) { - ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); - if (!ubs[i]) goto end; - argv[i] = ubs[i]->buf; - argvs[i] = ubs[i]->pos; - } + if (!uwsgi_rpc_apply_translations(wsgi_req, ur, ubs, &i, argv, argvs)) + goto end; // ok we now need to check it it is a local call or a remote one char *func = uwsgi_str(ur->data); - char *remote = NULL; - char *at = strchr(func, '@'); - if (at) { - *at = 0; - remote = at+1; - } + char *remote = uwsgi_rpc_get_remote(func); uint64_t size; response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func); @@ -509,28 +481,13 @@ static int uwsgi_routing_func_rpc_ret(struct wsgi_request *wsgi_req, struct uwsg // this is a placeholder for tmp uwsgi_buffers struct uwsgi_buffer *ubs[UMAX8]; - char **r_argv = (char **) ur->data2; - uint16_t *r_argvs = (uint16_t *) ur->data3; - - char **subject = (char **) (((char *)(wsgi_req))+ur->subject); - uint16_t *subject_len = (uint16_t *) (((char *)(wsgi_req))+ur->subject_len); - uint64_t i; - for(i=0;icustom;i++) { - ubs[i] = uwsgi_routing_translate(wsgi_req, ur, *subject, *subject_len, r_argv[i], r_argvs[i]); - if (!ubs[i]) goto end; - argv[i] = ubs[i]->buf; - argvs[i] = ubs[i]->pos; - } + if (!uwsgi_rpc_apply_translations(wsgi_req, ur, ubs, &i, argv, argvs)) + goto end; // ok we now need to check it it is a local call or a remote one char *func = uwsgi_str(ur->data); - char *remote = NULL; - char *at = strchr(func, '@'); - if (at) { - *at = 0; - remote = at+1; - } + char *remote = uwsgi_rpc_get_remote(func); uint64_t size; char *response = uwsgi_do_rpc(remote, func, ur->custom, argv, argvs, &size); free(func);