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
This commit is contained in:
Pau Espin 2021-06-10 13:15:20 +02:00 committed by pespin
parent 68ab9c4193
commit f5b8ed14a9
2 changed files with 34 additions and 62 deletions

View File

@ -516,92 +516,61 @@ err:
* \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.

View File

@ -117,11 +117,14 @@ static void assert_test(struct ctrl_handle *ctrl, struct ctrl_connection *ccon,
} else {
struct msgb *sent_msg = msgb_dequeue(&ccon->write_queue.msg_queue);
OSMO_ASSERT(sent_msg);
msgb_put_u8(sent_msg, 0);
printf("replied: '%s'\n", osmo_escape_str((char*)msgb_l2(sent_msg), -1));
char *strbuf = talloc_size(sent_msg, msgb_l2len(sent_msg) + 1);
memcpy(strbuf, msgb_l2(sent_msg), msgb_l2len(sent_msg));
strbuf[msgb_l2len(sent_msg)] = '\0';
printf("replied: '%s'\n", osmo_escape_str(strbuf, -1));
OSMO_ASSERT(t->reply_str);
OSMO_ASSERT(!strcmp(t->reply_str, (char*)msgb_l2(sent_msg)));
OSMO_ASSERT(!strcmp(t->reply_str, strbuf));
msgb_free(sent_msg);
}
osmo_wqueue_clear(&ccon->write_queue);