mirror of
https://github.com/opnsense/src.git
synced 2026-05-28 04:12:45 -04:00
libiscsiutil: Fix a memory leak with negotiation keys.
When keys are loaded from a received PDU, a copy of the received keys block is saved in the keys struct and the name and value pointers point into that saved block. Freeing the keys frees this block. However, when keys are added to a keys struct to build a set of keys later sent in a PDU, the keys data block pointer is not used and individual key names and values hold allocated strings. When the keys structure was freed, all of these individual key name and value strings were leaked. Instead, allocate copies of strings for names and values when parsing a set of keys from a received PDU and free all of the individual key name and value strings when deleting a set of keys. Reviewed by: mav Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D33545
This commit is contained in:
parent
6378393308
commit
fd99905b45
2 changed files with 17 additions and 15 deletions
|
|
@ -51,7 +51,10 @@ void
|
|||
keys_delete(struct keys *keys)
|
||||
{
|
||||
|
||||
free(keys->keys_data);
|
||||
for (int i = 0; i < KEYS_MAX; i++) {
|
||||
free(keys->keys_names[i]);
|
||||
free(keys->keys_values[i]);
|
||||
}
|
||||
free(keys);
|
||||
}
|
||||
|
||||
|
|
@ -59,7 +62,7 @@ void
|
|||
keys_load(struct keys *keys, const struct pdu *pdu)
|
||||
{
|
||||
int i;
|
||||
char *pair;
|
||||
char *keys_data, *name, *pair, *value;
|
||||
size_t pair_len;
|
||||
|
||||
if (pdu->pdu_data_len == 0)
|
||||
|
|
@ -68,35 +71,36 @@ keys_load(struct keys *keys, const struct pdu *pdu)
|
|||
if (pdu->pdu_data[pdu->pdu_data_len - 1] != '\0')
|
||||
log_errx(1, "protocol error: key not NULL-terminated\n");
|
||||
|
||||
assert(keys->keys_data == NULL);
|
||||
keys->keys_data_len = pdu->pdu_data_len;
|
||||
keys->keys_data = malloc(keys->keys_data_len);
|
||||
if (keys->keys_data == NULL)
|
||||
keys_data = malloc(pdu->pdu_data_len);
|
||||
if (keys_data == NULL)
|
||||
log_err(1, "malloc");
|
||||
memcpy(keys->keys_data, pdu->pdu_data, keys->keys_data_len);
|
||||
memcpy(keys_data, pdu->pdu_data, pdu->pdu_data_len);
|
||||
|
||||
/*
|
||||
* XXX: Review this carefully.
|
||||
*/
|
||||
pair = keys->keys_data;
|
||||
pair = keys_data;
|
||||
for (i = 0;; i++) {
|
||||
if (i >= KEYS_MAX)
|
||||
log_errx(1, "too many keys received");
|
||||
|
||||
pair_len = strlen(pair);
|
||||
|
||||
keys->keys_values[i] = pair;
|
||||
keys->keys_names[i] = strsep(&keys->keys_values[i], "=");
|
||||
if (keys->keys_names[i] == NULL || keys->keys_values[i] == NULL)
|
||||
value = pair;
|
||||
name = strsep(&value, "=");
|
||||
if (name == NULL || value == NULL)
|
||||
log_errx(1, "malformed keys");
|
||||
keys->keys_names[i] = checked_strdup(name);
|
||||
keys->keys_values[i] = checked_strdup(value);
|
||||
log_debugx("key received: \"%s=%s\"",
|
||||
keys->keys_names[i], keys->keys_values[i]);
|
||||
|
||||
pair += pair_len + 1; /* +1 to skip the terminating '\0'. */
|
||||
if (pair == keys->keys_data + keys->keys_data_len)
|
||||
if (pair == keys_data + pdu->pdu_data_len)
|
||||
break;
|
||||
assert(pair < keys->keys_data + keys->keys_data_len);
|
||||
assert(pair < keys_data + pdu->pdu_data_len);
|
||||
}
|
||||
free(keys_data);
|
||||
}
|
||||
|
||||
void
|
||||
|
|
|
|||
|
|
@ -75,8 +75,6 @@ struct connection_ops {
|
|||
struct keys {
|
||||
char *keys_names[KEYS_MAX];
|
||||
char *keys_values[KEYS_MAX];
|
||||
char *keys_data;
|
||||
size_t keys_data_len;
|
||||
};
|
||||
|
||||
#define CHAP_CHALLENGE_LEN 1024
|
||||
|
|
|
|||
Loading…
Reference in a new issue