ctrl: Pre-calculate required size before allocating msgb
This commit fixes crash when response is more than ~4096 chars.
Furthermore, we now allocate only the required memory, not 4096 for all
messages, which usually don't require it.
Test needs to be adapted since it assumed there was more available space
at the end of the msgb.
Related: OS#5169
Change-Id: I0b8f370f7b08736207f9efed13a0663b5e482824
diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c
index 33496bd..753e6cc 100644
--- a/src/ctrl/control_cmd.c
+++ b/src/ctrl/control_cmd.c
@@ -516,92 +516,61 @@
* \returns callee-allocated message buffer containing the encoded \a cmd; NULL on error */
struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd)
{
- struct msgb *msg;
+ struct msgb *msg = NULL;
+ char *strbuf;
+ size_t len;
const char *type;
- char *tmp;
if (!cmd->id)
return NULL;
- msg = msgb_alloc_headroom(4096, 128, "ctrl command make");
- if (!msg)
- return NULL;
-
type = get_value_string(ctrl_type_vals, cmd->type);
switch (cmd->type) {
case CTRL_TYPE_GET:
if (!cmd->variable)
- goto err;
-
- tmp = talloc_asprintf(cmd, "%s %s %s", type, cmd->id, cmd->variable);
- if (!tmp) {
- LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
- goto err;
- }
-
- msg->l2h = msgb_put(msg, strlen(tmp));
- memcpy(msg->l2h, tmp, strlen(tmp));
- talloc_free(tmp);
+ return NULL;
+ strbuf = talloc_asprintf(cmd, "%s %s %s", type, cmd->id, cmd->variable);
break;
case CTRL_TYPE_SET:
if (!cmd->variable || !cmd->value)
- goto err;
-
- tmp = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id, cmd->variable,
- cmd->value);
- if (!tmp) {
- LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
- goto err;
- }
-
- msg->l2h = msgb_put(msg, strlen(tmp));
- memcpy(msg->l2h, tmp, strlen(tmp));
- talloc_free(tmp);
+ return NULL;
+ strbuf = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id,
+ cmd->variable, cmd->value);
break;
case CTRL_TYPE_GET_REPLY:
case CTRL_TYPE_SET_REPLY:
case CTRL_TYPE_TRAP:
if (!cmd->variable || !cmd->reply)
- goto err;
-
- tmp = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id, cmd->variable,
- cmd->reply);
- if (!tmp) {
- LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
- goto err;
- }
-
- msg->l2h = msgb_put(msg, strlen(tmp));
- memcpy(msg->l2h, tmp, strlen(tmp));
- talloc_free(tmp);
+ return NULL;
+ strbuf = talloc_asprintf(cmd, "%s %s %s %s", type, cmd->id,
+ cmd->variable, cmd->reply);
break;
case CTRL_TYPE_ERROR:
if (!cmd->reply)
- goto err;
-
- tmp = talloc_asprintf(cmd, "%s %s %s", type, cmd->id,
- cmd->reply);
- if (!tmp) {
- LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
- goto err;
- }
-
- msg->l2h = msgb_put(msg, strlen(tmp));
- memcpy(msg->l2h, tmp, strlen(tmp));
- talloc_free(tmp);
+ return NULL;
+ strbuf = talloc_asprintf(cmd, "%s %s %s", type, cmd->id, cmd->reply);
break;
default:
LOGP(DLCTRL, LOGL_NOTICE, "Unknown command type %i\n", cmd->type);
- goto err;
- break;
+ return NULL;
}
- return msg;
+ if (!strbuf) {
+ LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate cmd.\n");
+ goto ret;
+ }
+ len = strlen(strbuf);
-err:
- msgb_free(msg);
- return NULL;
+ msg = msgb_alloc_headroom(len + 128, 128, "ctrl ERROR command make");
+ if (!msg)
+ goto ret;
+ msg->l2h = msgb_put(msg, len);
+ memcpy(msg->l2h, strbuf, len);
+
+ret:
+ talloc_free(strbuf);
+ return msg;
}
/*! Build a deferred control command state and keep it the per-connection list of deferred commands.