<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7240">View Change</a></p><div style="white-space:pre-wrap">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
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">acl: Fix allocation related issues.<br><br>Add checks for allocation errors, cleanup and report failure when they<br>occur.<br><br>* ast_duplicate_acl_list: Replace log warnings with errors, add missing<br> line-feed.<br>* ast_append_acl: Add missing line-feed to logger message.<br>* ast_append_ha: Avoid ast_strdupa in loop by moving debug message to<br> separate function.<br>* ast_ha_join: Use two separate calls to ast_str_append to avoid using<br> ast_strdupa in a loop.<br><br>Change-Id: Ia19eaaeb0b139ff7ce7b971c7550e85c8b78ab76<br>---<br>M main/acl.c<br>1 file changed, 48 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/acl.c b/main/acl.c<br>index 6868ea1..bcb3f63 100644<br>--- a/main/acl.c<br>+++ b/main/acl.c<br>@@ -281,6 +281,12 @@<br> <br> while (start) {<br> current = ast_duplicate_ha(start); /* Create copy of this object */<br>+ if (!current) {<br>+ ast_free_ha(ret);<br>+<br>+ return NULL;<br>+ }<br>+<br> if (prev) {<br> prev->next = current; /* Link previous to this object */<br> }<br>@@ -318,7 +324,7 @@<br> }<br> <br> if (!(clone = ast_calloc(1, sizeof(*clone)))) {<br>- ast_log(LOG_WARNING, "Failed to allocate ast_acl_list struct while cloning an ACL\n");<br>+ ast_log(LOG_ERROR, "Failed to allocate ast_acl_list struct while cloning an ACL\n");<br> return NULL;<br> }<br> AST_LIST_HEAD_INIT(clone);<br>@@ -327,8 +333,10 @@<br> <br> AST_LIST_TRAVERSE(original, current_cursor, list) {<br> if ((acl_new(¤t_clone, current_cursor->name))) {<br>- ast_log(LOG_WARNING, "Failed to allocate ast_acl struct while cloning an ACL.");<br>- continue;<br>+ ast_log(LOG_ERROR, "Failed to allocate ast_acl struct while cloning an ACL.\n");<br>+ ast_free_acl_list(clone);<br>+ clone = NULL;<br>+ break;<br> }<br> <br> /* Copy data from original ACL to clone ACL */<br>@@ -338,6 +346,15 @@<br> current_clone->is_realtime = current_cursor->is_realtime;<br> <br> AST_LIST_INSERT_TAIL(clone, current_clone, list);<br>+<br>+ if (current_cursor->acl && !current_clone->acl) {<br>+ /* Deal with failure after adding to clone so we don't have to free<br>+ * current_clone separately. */<br>+ ast_log(LOG_ERROR, "Failed to duplicate HA list while cloning ACL.\n");<br>+ ast_free_acl_list(clone);<br>+ clone = NULL;<br>+ break;<br>+ }<br> }<br> <br> AST_LIST_UNLOCK(original);<br>@@ -448,6 +465,8 @@<br> if (error) {<br> *error = 1;<br> }<br>+ AST_LIST_UNLOCK(working_list);<br>+ return;<br> }<br> // Need to INSERT the ACL at the head here.<br> AST_LIST_INSERT_HEAD(working_list, acl, list);<br>@@ -477,7 +496,8 @@<br> AST_LIST_TRAVERSE(working_list, current, list) {<br> if (!strcasecmp(current->name, tmp)) { /* ACL= */<br> /* Inclusion of the same ACL multiple times isn't a catastrophic error, but it will raise the error flag and skip the entry. */<br>- ast_log(LOG_ERROR, "Named ACL '%s' occurs multiple times in ACL definition. Please update your ACL configuration.", tmp);<br>+ ast_log(LOG_ERROR, "Named ACL '%s' occurs multiple times in ACL definition. "<br>+ "Please update your ACL configuration.\n", tmp);<br> if (error) {<br> *error = 1;<br> }<br>@@ -536,6 +556,22 @@<br> return 1;<br> }<br> <br>+/*!<br>+ * \internal<br>+ * \brief Used by ast_append_ha to avoid ast_strdupa in a loop.<br>+ *<br>+ * \note This function is only called at debug level 3 and higher.<br>+ */<br>+static void debug_ha_sense_appended(struct ast_ha *ha)<br>+{<br>+ const char *parsed_mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));<br>+<br>+ ast_log(LOG_DEBUG, "%s/%s sense %u appended to ACL\n",<br>+ ast_sockaddr_stringify(&ha->addr),<br>+ parsed_mask,<br>+ ha->sense);<br>+}<br>+<br> struct ast_ha *ast_append_ha(const char *sense, const char *stuff, struct ast_ha *path, int *error)<br> {<br> struct ast_ha *ha;<br>@@ -545,7 +581,6 @@<br> char *address = NULL, *mask = NULL;<br> int addr_is_v4;<br> int allowing = strncasecmp(sense, "p", 1) ? AST_SENSE_DENY : AST_SENSE_ALLOW;<br>- const char *parsed_addr, *parsed_mask;<br> <br> ret = path;<br> while (path) {<br>@@ -653,10 +688,9 @@<br> }<br> prev = ha;<br> <br>- parsed_addr = ast_strdupa(ast_sockaddr_stringify(&ha->addr));<br>- parsed_mask = ast_strdupa(ast_sockaddr_stringify(&ha->netmask));<br>-<br>- ast_debug(3, "%s/%s sense %u appended to ACL\n", parsed_addr, parsed_mask, ha->sense);<br>+ if (DEBUG_ATLEAST(3)) {<br>+ debug_ha_sense_appended(ha);<br>+ }<br> }<br> <br> return ret;<br>@@ -665,10 +699,11 @@<br> void ast_ha_join(const struct ast_ha *ha, struct ast_str **buf)<br> {<br> for (; ha; ha = ha->next) {<br>- const char *addr = ast_strdupa(ast_sockaddr_stringify_addr(&ha->addr));<br>- ast_str_append(buf, 0, "%s%s/%s",<br>- ha->sense == AST_SENSE_ALLOW ? "!" : "",<br>- addr, ast_sockaddr_stringify_addr(&ha->netmask));<br>+ ast_str_append(buf, 0, "%s%s/",<br>+ ha->sense == AST_SENSE_ALLOW ? "!" : "",<br>+ ast_sockaddr_stringify_addr(&ha->addr));<br>+ /* Separated to avoid duplicating stringified addresses. */<br>+ ast_str_append(buf, 0, "%s", ast_sockaddr_stringify_addr(&ha->netmask));<br> if (ha->next) {<br> ast_str_append(buf, 0, ",");<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7240">change 7240</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7240"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ia19eaaeb0b139ff7ce7b971c7550e85c8b78ab76 </div>
<div style="display:none"> Gerrit-Change-Number: 7240 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>