From adcbdb1d40c7092e472ebf270b43c5377c98792a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 19 Jan 2022 15:57:08 +0100 Subject: [PATCH 1/2] s4:dsdb/paged_results: fix segfault in paged_results() It can happen that the paged_results() failes, e.g. due to LDB_ERR_TIME_LIMIT_EXCEEDED, if that happens we should not dereference ares->response, if ares is NULL. We also should not call ldb_module_done() if paged_results() fails, as it was already called. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14952 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke Reviewed-by: Douglas Bagnall (cherry picked from commit 19fa22b1fbcf33dbc4defe4dd2e487a642786c49) --- .../dsdb/samdb/ldb_modules/paged_results.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c b/source4/dsdb/samdb/ldb_modules/paged_results.c index 3eea3236e7dc..2063e84e1579 100644 --- a/source4/dsdb/samdb/ldb_modules/paged_results.c +++ b/source4/dsdb/samdb/ldb_modules/paged_results.c @@ -239,6 +239,7 @@ static int paged_search_by_dn_guid(struct ldb_module *module, static int paged_results(struct paged_context *ac, struct ldb_reply *ares) { + struct ldb_extended *response = (ares != NULL ? ares->response : NULL); struct ldb_paged_control *paged; unsigned int i, num_ctrls; int ret; @@ -246,7 +247,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) if (ac->store == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } while (ac->store->last_i < ac->store->num_entries && ac->size > 0) { @@ -276,7 +277,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) continue; } else if (ret != LDB_SUCCESS) { return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ret = ldb_module_send_entry(ac->req, result->msgs[0], @@ -318,7 +319,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) if (ac->controls == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[num_ctrls] = NULL; @@ -331,7 +332,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) if (ac->controls[i] == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->oid = talloc_strdup(ac->controls[i], @@ -339,7 +340,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) if (ac->controls[i]->oid == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->critical = 0; @@ -348,7 +349,7 @@ static int paged_results(struct paged_context *ac, struct ldb_reply *ares) if (paged == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->data = paged; @@ -803,7 +804,11 @@ static int paged_search(struct ldb_module *module, struct ldb_request *req) ret = paged_results(ac, NULL); if (ret != LDB_SUCCESS) { - return ldb_module_done(req, NULL, NULL, ret); + /* + * paged_results() will have called ldb_module_done + * if an error occurred + */ + return ret; } return ldb_module_done(req, ac->controls, NULL, LDB_SUCCESS); } -- 2.25.1 From 4c46b8cbfb5224f789edccd922da73c594415e61 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 19 Jan 2022 15:57:08 +0100 Subject: [PATCH 2/2] s4:dsdb/vlv_pagination: fix segfault in vlv_results() It can happen that the vlv_results() failes, e.g. due to LDB_ERR_TIME_LIMIT_EXCEEDED, if that happens we should not dereference ares->response, if ares is NULL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14952 Signed-off-by: Stefan Metzmacher Reviewed-by: Douglas Bagnall Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Jan 20 10:04:39 UTC 2022 on sn-devel-184 (cherry picked from commit 7d16a56b9d1cde8a5174381ef4924a2ea7be59bc) --- .../dsdb/samdb/ldb_modules/vlv_pagination.c | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c index d6d6039e8492..b389d3fd4f0b 100644 --- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c +++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c @@ -389,6 +389,7 @@ static int vlv_calc_real_offset(int offset, int denominator, int n_entries) static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) { + struct ldb_extended *response = (ares != NULL ? ares->response : NULL); struct ldb_vlv_resp_control *vlv; unsigned int num_ctrls; int ret, i, first_i, last_i; @@ -399,7 +400,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) if (ac->store == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } if (ac->store->first_ref) { @@ -428,7 +429,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) return ldb_module_done( ac->req, ac->controls, - ares->response, + response, ret); } } else { @@ -440,7 +441,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) return ldb_module_done( ac->req, ac->controls, - ares->response, + response, ret); } } @@ -480,7 +481,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) return ldb_module_done( ac->req, ac->controls, - ares->response, + response, ret); } @@ -513,7 +514,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) if (ac->controls == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[num_ctrls] = NULL; @@ -525,7 +526,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) if (ac->controls[i] == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->oid = talloc_strdup(ac->controls[i], @@ -533,7 +534,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) if (ac->controls[i]->oid == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->critical = 0; @@ -542,7 +543,7 @@ static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) if (vlv == NULL) { ret = LDB_ERR_OPERATIONS_ERROR; return ldb_module_done( - ac->req, ac->controls, ares->response, ret); + ac->req, ac->controls, response, ret); } ac->controls[i]->data = vlv; @@ -891,6 +892,10 @@ static int vlv_search(struct ldb_module *module, struct ldb_request *req) ret = vlv_results(ac, NULL); if (ret != LDB_SUCCESS) { + /* + * vlv_results() will have called ldb_module_done + * if there was an error. + */ return ret; } return ldb_module_done(req, ac->controls, NULL, -- 2.25.1