URL: https://github.com/SSSD/sssd/pull/211 Author: celestian Title: #211: IFP: Fix of limit = 0 (unlimited result) Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/211/head:pr211 git checkout pr211
From 06308fbd29c2516b259ac65d326c4591284d59d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C4=8Cech?= <[email protected]> Date: Thu, 23 Mar 2017 09:17:55 +0100 Subject: [PATCH 1/3] IFP: Filter with * in infopipe group methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch fixes asterisk in filter of the ListByName Groups' method, which ends up calling ifp_groups_list_copy() with a NULL pointer. Resolves: https://pagure.io/SSSD/sssd/issue/3305 Reviewed-by: Fabiano Fidêncio <[email protected]> --- src/responder/ifp/ifp_groups.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/responder/ifp/ifp_groups.c b/src/responder/ifp/ifp_groups.c index c568c62..5c126fc 100644 --- a/src/responder/ifp/ifp_groups.c +++ b/src/responder/ifp/ifp_groups.c @@ -307,12 +307,14 @@ static void ifp_groups_list_by_name_done(struct tevent_req *req) return; } - ret = ifp_groups_list_copy(list_ctx, result->ldb_result); - if (ret != EOK) { - error = sbus_error_new(sbus_req, SBUS_ERROR_INTERNAL, - "Failed to copy domain result"); - sbus_request_fail_and_finish(sbus_req, error); - return; + if (ret == EOK) { + ret = ifp_groups_list_copy(list_ctx, result->ldb_result); + if (ret != EOK) { + error = sbus_error_new(sbus_req, SBUS_ERROR_INTERNAL, + "Failed to copy domain result"); + sbus_request_fail_and_finish(sbus_req, error); + return; + } } list_ctx->dom = get_next_domain(list_ctx->dom, SSS_GND_DESCEND); @@ -394,11 +396,13 @@ static void ifp_groups_list_by_domain_and_name_done(struct tevent_req *req) goto done; } - ret = ifp_groups_list_copy(list_ctx, result->ldb_result); - if (ret != EOK) { - error = sbus_error_new(sbus_req, SBUS_ERROR_INTERNAL, - "Failed to copy domain result"); - goto done; + if (ret == EOK) { + ret = ifp_groups_list_copy(list_ctx, result->ldb_result); + if (ret != EOK) { + error = sbus_error_new(sbus_req, SBUS_ERROR_INTERNAL, + "Failed to copy domain result"); + goto done; + } } done: From b3d565cdfa905d0030e876ec0b91fcc5f4b2d36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C4=8Cech?= <[email protected]> Date: Tue, 28 Mar 2017 12:07:55 +0200 Subject: [PATCH 2/3] IFP: Fix of limit = 0 (unlimited result) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we set limit to 0 it means that result is unlimited. Internally we restrict number of result by allocation of result array. In unlimited case there was a bug and zero array was allocated. This fix allocates neccessary array when we know real result size. Resolves: https://pagure.io/SSSD/sssd/issue/3306 Reviewed-by: Fabiano Fidêncio <[email protected]> --- src/responder/ifp/ifpsrv_util.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c index 5866d30..c948d5a 100644 --- a/src/responder/ifp/ifpsrv_util.c +++ b/src/responder/ifp/ifpsrv_util.c @@ -314,6 +314,15 @@ size_t ifp_list_ctx_remaining_capacity(struct ifp_list_ctx *list_ctx, { size_t capacity = list_ctx->limit - list_ctx->path_count; + if (list_ctx->limit == 0) { + list_ctx->paths = talloc_zero_array(list_ctx, const char *, entries); + if (list_ctx->paths == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero_array() failed\n"); + return 0; + } + return entries; + } + if (capacity < entries) { DEBUG(SSSDBG_MINOR_FAILURE, "IFP list request has limit of %"PRIu32" entries but back end " From 0a8916bcc3b63768c66fbf94021d3fc50badc3f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> Date: Fri, 5 May 2017 10:38:41 +0200 Subject: [PATCH 3/3] IFP: Change ifp_list_ctx_remaining_capacity() return type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now ifp_list_ctx_remaining_capacity() returns an errno_t and receives the count as an output parameter. It allows better handling and error reporting in case something goes wrong internally in this function. Related: https://pagure.io/SSSD/sssd/issue/3306 Signed-off-by: Fabiano Fidêncio <[email protected]> --- src/responder/ifp/ifp_groups.c | 14 +++++++++++--- src/responder/ifp/ifp_private.h | 5 +++-- src/responder/ifp/ifp_users.c | 21 +++++++++++++++++---- src/responder/ifp/ifpsrv_util.c | 24 ++++++++++++++++++------ 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/responder/ifp/ifp_groups.c b/src/responder/ifp/ifp_groups.c index 5c126fc..e9dcbe5 100644 --- a/src/responder/ifp/ifp_groups.c +++ b/src/responder/ifp/ifp_groups.c @@ -87,8 +87,12 @@ static int ifp_groups_list_copy(struct ifp_list_ctx *list_ctx, struct ldb_result *result) { size_t copy_count, i; + errno_t ret; - copy_count = ifp_list_ctx_remaining_capacity(list_ctx, result->count); + ret = ifp_list_ctx_remaining_capacity(list_ctx, result->count, ©_count); + if (ret != EOK) { + goto done; + } for (i = 0; i < copy_count; i++) { list_ctx->paths[list_ctx->path_count + i] = \ @@ -96,12 +100,16 @@ static int ifp_groups_list_copy(struct ifp_list_ctx *list_ctx, list_ctx->dom, result->msgs[i]); if (list_ctx->paths[list_ctx->path_count + i] == NULL) { - return ENOMEM; + ret = ENOMEM; + goto done; } } list_ctx->path_count += copy_count; - return EOK; + ret = EOK; + +done: + return ret; } static void ifp_groups_find_by_name_done(struct tevent_req *req); diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h index e800070..d7e8b75 100644 --- a/src/responder/ifp/ifp_private.h +++ b/src/responder/ifp/ifp_private.h @@ -99,8 +99,9 @@ struct ifp_list_ctx *ifp_list_ctx_new(struct sbus_request *sbus_req, const char *filter, uint32_t limit); -size_t ifp_list_ctx_remaining_capacity(struct ifp_list_ctx *list_ctx, - size_t entries); +errno_t ifp_list_ctx_remaining_capacity(struct ifp_list_ctx *list_ctx, + size_t entries, + size_t *_capacity); errno_t ifp_ldb_el_output_name(struct resp_ctx *rctx, struct ldb_message *msg, diff --git a/src/responder/ifp/ifp_users.c b/src/responder/ifp/ifp_users.c index 188194f..0159062 100644 --- a/src/responder/ifp/ifp_users.c +++ b/src/responder/ifp/ifp_users.c @@ -436,8 +436,12 @@ static int ifp_users_list_copy(struct ifp_list_ctx *list_ctx, struct ldb_result *result) { size_t copy_count, i; + errno_t ret; - copy_count = ifp_list_ctx_remaining_capacity(list_ctx, result->count); + ret = ifp_list_ctx_remaining_capacity(list_ctx, result->count, ©_count); + if (ret != EOK) { + goto done; + } for (i = 0; i < copy_count; i++) { list_ctx->paths[list_ctx->path_count + i] = \ @@ -445,12 +449,16 @@ static int ifp_users_list_copy(struct ifp_list_ctx *list_ctx, list_ctx->dom, result->msgs[i]); if (list_ctx->paths[list_ctx->path_count + i] == NULL) { - return ENOMEM; + ret = ENOMEM; + goto done; } } list_ctx->path_count += copy_count; - return EOK; + ret = EOK; + +done: + return ret; } struct name_and_cert_ctx { @@ -906,7 +914,12 @@ static void ifp_users_list_by_domain_and_name_done(struct tevent_req *req) goto done; } - copy_count = ifp_list_ctx_remaining_capacity(list_ctx, result->count); + ret = ifp_list_ctx_remaining_capacity(list_ctx, result->count, ©_count); + if (ret != EOK) { + error = sbus_error_new(sbus_req, SBUS_ERROR_INTERNAL, + "Failed to get the list remaining capacity\n"); + goto done; + } for (i = 0; i < copy_count; i++) { list_ctx->paths[i] = ifp_users_build_path_from_msg(list_ctx->paths, diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c index c948d5a..dcb9fd5 100644 --- a/src/responder/ifp/ifpsrv_util.c +++ b/src/responder/ifp/ifpsrv_util.c @@ -309,28 +309,40 @@ struct ifp_list_ctx *ifp_list_ctx_new(struct sbus_request *sbus_req, return list_ctx; } -size_t ifp_list_ctx_remaining_capacity(struct ifp_list_ctx *list_ctx, - size_t entries) +errno_t ifp_list_ctx_remaining_capacity(struct ifp_list_ctx *list_ctx, + size_t entries, + size_t *_capacity) { size_t capacity = list_ctx->limit - list_ctx->path_count; + errno_t ret; if (list_ctx->limit == 0) { list_ctx->paths = talloc_zero_array(list_ctx, const char *, entries); if (list_ctx->paths == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero_array() failed\n"); - return 0; + ret = ENOMEM; + goto done; } - return entries; + capacity = entries; + goto immediately; } if (capacity < entries) { DEBUG(SSSDBG_MINOR_FAILURE, "IFP list request has limit of %"PRIu32" entries but back end " "returned %zu entries\n", list_ctx->limit, entries); - return capacity; + goto immediately; } else { - return entries; + capacity = entries; + goto immediately; } + +immediately: + *_capacity = capacity; + ret = EOK; + +done: + return ret; } errno_t ifp_ldb_el_output_name(struct resp_ctx *rctx,
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
