[Asterisk-code-review] acl: Fix allocation related issues. (asterisk[13])
Joshua Colp
asteriskteam at digium.com
Mon Nov 20 16:48:16 CST 2017
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/7239 )
Change subject: acl: Fix allocation related issues.
......................................................................
acl: Fix allocation related issues.
Add checks for allocation errors, cleanup and report failure when they
occur.
* ast_duplicate_acl_list: Replace log warnings with errors, add missing
line-feed.
* ast_append_acl: Add missing line-feed to logger message.
* ast_append_ha: Avoid ast_strdupa in loop by moving debug message to
separate function.
* ast_ha_join: Use two separate calls to ast_str_append to avoid using
ast_strdupa in a loop.
Change-Id: Ia19eaaeb0b139ff7ce7b971c7550e85c8b78ab76
---
M main/acl.c
1 file changed, 48 insertions(+), 13 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/main/acl.c b/main/acl.c
index d4b3089..4303532 100644
--- a/main/acl.c
+++ b/main/acl.c
@@ -283,6 +283,12 @@
while (start) {
current = ast_duplicate_ha(start); /* Create copy of this object */
+ if (!current) {
+ ast_free_ha(ret);
+
+ return NULL;
+ }
+
if (prev) {
prev->next = current; /* Link previous to this object */
}
@@ -320,7 +326,7 @@
}
if (!(clone = ast_calloc(1, sizeof(*clone)))) {
- ast_log(LOG_WARNING, "Failed to allocate ast_acl_list struct while cloning an ACL\n");
+ ast_log(LOG_ERROR, "Failed to allocate ast_acl_list struct while cloning an ACL\n");
return NULL;
}
AST_LIST_HEAD_INIT(clone);
@@ -329,8 +335,10 @@
AST_LIST_TRAVERSE(original, current_cursor, list) {
if ((acl_new(¤t_clone, current_cursor->name))) {
- ast_log(LOG_WARNING, "Failed to allocate ast_acl struct while cloning an ACL.");
- continue;
+ ast_log(LOG_ERROR, "Failed to allocate ast_acl struct while cloning an ACL.\n");
+ ast_free_acl_list(clone);
+ clone = NULL;
+ break;
}
/* Copy data from original ACL to clone ACL */
@@ -340,6 +348,15 @@
current_clone->is_realtime = current_cursor->is_realtime;
AST_LIST_INSERT_TAIL(clone, current_clone, list);
+
+ if (current_cursor->acl && !current_clone->acl) {
+ /* Deal with failure after adding to clone so we don't have to free
+ * current_clone separately. */
+ ast_log(LOG_ERROR, "Failed to duplicate HA list while cloning ACL.\n");
+ ast_free_acl_list(clone);
+ clone = NULL;
+ break;
+ }
}
AST_LIST_UNLOCK(original);
@@ -450,6 +467,8 @@
if (error) {
*error = 1;
}
+ AST_LIST_UNLOCK(working_list);
+ return;
}
// Need to INSERT the ACL at the head here.
AST_LIST_INSERT_HEAD(working_list, acl, list);
@@ -479,7 +498,8 @@
AST_LIST_TRAVERSE(working_list, current, list) {
if (!strcasecmp(current->name, tmp)) { /* ACL= */
/* Inclusion of the same ACL multiple times isn't a catastrophic error, but it will raise the error flag and skip the entry. */
- ast_log(LOG_ERROR, "Named ACL '%s' occurs multiple times in ACL definition. Please update your ACL configuration.", tmp);
+ ast_log(LOG_ERROR, "Named ACL '%s' occurs multiple times in ACL definition. "
+ "Please update your ACL configuration.\n", tmp);
if (error) {
*error = 1;
}
@@ -538,6 +558,22 @@
return 1;
}
+/*!
+ * \internal
+ * \brief Used by ast_append_ha to avoid ast_strdupa in a loop.
+ *
+ * \note This function is only called at debug level 3 and higher.
+ */
+static void debug_ha_sense_appended(struct ast_ha *ha)
+{
+ const char *parsed_mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
+
+ ast_log(LOG_DEBUG, "%s/%s sense %u appended to ACL\n",
+ ast_sockaddr_stringify(&ha->addr),
+ parsed_mask,
+ ha->sense);
+}
+
struct ast_ha *ast_append_ha(const char *sense, const char *stuff, struct ast_ha *path, int *error)
{
struct ast_ha *ha;
@@ -547,7 +583,6 @@
char *address = NULL, *mask = NULL;
int addr_is_v4;
int allowing = strncasecmp(sense, "p", 1) ? AST_SENSE_DENY : AST_SENSE_ALLOW;
- const char *parsed_addr, *parsed_mask;
ret = path;
while (path) {
@@ -655,10 +690,9 @@
}
prev = ha;
- parsed_addr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));
- parsed_mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));
-
- ast_debug(3, "%s/%s sense %u appended to ACL\n", parsed_addr, parsed_mask, ha->sense);
+ if (DEBUG_ATLEAST(3)) {
+ debug_ha_sense_appended(ha);
+ }
}
return ret;
@@ -667,10 +701,11 @@
void ast_ha_join(const struct ast_ha *ha, struct ast_str **buf)
{
for (; ha; ha = ha->next) {
- const char *addr = ast_strdupa(ast_sockaddr_stringify_addr(&ha->addr));
- ast_str_append(buf, 0, "%s%s/%s",
- ha->sense == AST_SENSE_ALLOW ? "!" : "",
- addr, ast_sockaddr_stringify_addr(&ha->netmask));
+ ast_str_append(buf, 0, "%s%s/",
+ ha->sense == AST_SENSE_ALLOW ? "!" : "",
+ ast_sockaddr_stringify_addr(&ha->addr));
+ /* Separated to avoid duplicating stringified addresses. */
+ ast_str_append(buf, 0, "%s", ast_sockaddr_stringify_addr(&ha->netmask));
if (ha->next) {
ast_str_append(buf, 0, ",");
}
--
To view, visit https://gerrit.asterisk.org/7239
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia19eaaeb0b139ff7ce7b971c7550e85c8b78ab76
Gerrit-Change-Number: 7239
Gerrit-PatchSet: 4
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171120/44415aab/attachment-0001.html>
More information about the asterisk-code-review
mailing list