Fix memory leak in serial_read_packet_client (#80)

* Fix memory leak in serial_read_packet_client

* Fixing more memory leaks in `prio/serial.c`

The `UP_CHECK` macro just returned on failure, rather than
jumping to the `cleanup` label. This commit replaces `UP_CHECK`
with `UP_CHECKC`, which jumps to `cleanup` on failure.

* Replace `P_CHECK` with `P_CHECKC` in prio/serial.c

Every function in `prio/serial.c` has a `cleanup` label, but
much of the code in these functions won't actually jump to
cleanup on failure. As far as I can tell, this doesn't cause
any additional memory leaks, but in case someone adds cleanup
code at the `cleanup` label in the future, it seems prudent to
always jump to `cleanup` on failure.

Co-authored-by: Henry Corrigan-Gibbs <henrycg@csail.mit.edu>
This commit is contained in:
Christian Holler (:decoder) 2020-04-21 19:07:56 +02:00 коммит произвёл GitHub
Родитель 6efb51c538
Коммит ed4142ae1c
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 55 добавлений и 51 удалений

Просмотреть файл

@ -66,7 +66,7 @@ serial_read_mp_int(msgpack_unpacker* upk, mp_int* n, const mp_int* max)
P_CHECKCB(n != NULL);
P_CHECKCB(max != NULL);
UP_CHECK(msgpack_unpacker_next(upk, &res))
UP_CHECKC(msgpack_unpacker_next(upk, &res));
msgpack_object obj = res.data;
P_CHECKC(object_to_mp_int(&obj, n, max));
@ -88,7 +88,7 @@ serial_read_int(msgpack_unpacker* upk, int* n)
P_CHECKCB(upk != NULL);
P_CHECKCB(n != NULL);
UP_CHECK(msgpack_unpacker_next(upk, &res))
UP_CHECKC(msgpack_unpacker_next(upk, &res));
msgpack_object obj = res.data;
P_CHECKCB(obj.type == MSGPACK_OBJECT_POSITIVE_INTEGER);
@ -108,9 +108,9 @@ serial_write_mp_array(msgpack_packer* pk, const_MPArray arr)
P_CHECKCB(pk != NULL);
P_CHECKCB(arr != NULL);
P_CHECK(msgpack_pack_array(pk, arr->len));
P_CHECKC(msgpack_pack_array(pk, arr->len));
for (int i = 0; i < arr->len; i++) {
P_CHECK(serial_write_mp_int(pk, &arr->data[i]));
P_CHECKC(serial_write_mp_int(pk, &arr->data[i]));
}
cleanup:
@ -130,7 +130,7 @@ serial_read_mp_array(msgpack_unpacker* upk, MPArray arr, size_t len,
P_CHECKCB(arr != NULL);
P_CHECKCB(max != NULL);
UP_CHECK(msgpack_unpacker_next(upk, &res))
UP_CHECKC(msgpack_unpacker_next(upk, &res));
msgpack_object obj = res.data;
P_CHECKCB(obj.type == MSGPACK_OBJECT_ARRAY);
@ -156,9 +156,9 @@ serial_write_beaver_triple(msgpack_packer* pk, const_BeaverTriple t)
P_CHECKCB(pk != NULL);
P_CHECKCB(t != NULL);
P_CHECK(serial_write_mp_int(pk, &t->a));
P_CHECK(serial_write_mp_int(pk, &t->b));
P_CHECK(serial_write_mp_int(pk, &t->c));
P_CHECKC(serial_write_mp_int(pk, &t->a));
P_CHECKC(serial_write_mp_int(pk, &t->b));
P_CHECKC(serial_write_mp_int(pk, &t->c));
cleanup:
return rv;
@ -173,9 +173,9 @@ serial_read_beaver_triple(msgpack_unpacker* pk, BeaverTriple t,
P_CHECKCB(t != NULL);
P_CHECKCB(max != NULL);
P_CHECK(serial_read_mp_int(pk, &t->a, max));
P_CHECK(serial_read_mp_int(pk, &t->b, max));
P_CHECK(serial_read_mp_int(pk, &t->c, max));
P_CHECKC(serial_read_mp_int(pk, &t->a, max));
P_CHECKC(serial_read_mp_int(pk, &t->b, max));
P_CHECKC(serial_read_mp_int(pk, &t->c, max));
cleanup:
return rv;
@ -188,8 +188,8 @@ serial_write_server_a_data(msgpack_packer* pk, const struct server_a_data* A)
P_CHECKCB(pk != NULL);
P_CHECKCB(A != NULL);
P_CHECK(serial_write_mp_array(pk, A->data_shares));
P_CHECK(serial_write_mp_array(pk, A->h_points));
P_CHECKC(serial_write_mp_array(pk, A->data_shares));
P_CHECKC(serial_write_mp_array(pk, A->h_points));
cleanup:
return rv;
}
@ -202,9 +202,9 @@ serial_read_server_a_data(msgpack_unpacker* upk, struct server_a_data* A,
P_CHECKCB(upk != NULL);
P_CHECKCB(A != NULL);
P_CHECK(serial_read_mp_array(upk, A->data_shares, cfg->num_data_fields,
P_CHECKC(serial_read_mp_array(upk, A->data_shares, cfg->num_data_fields,
&cfg->modulus));
P_CHECK(serial_read_mp_array(upk, A->h_points, PrioConfig_hPoints(cfg),
P_CHECKC(serial_read_mp_array(upk, A->h_points, PrioConfig_hPoints(cfg),
&cfg->modulus));
cleanup:
@ -218,8 +218,8 @@ serial_write_prg_seed(msgpack_packer* pk, const PrioPRGSeed* seed)
P_CHECKCB(pk != NULL);
P_CHECKCB(seed != NULL);
P_CHECK(msgpack_pack_str(pk, PRG_SEED_LENGTH));
P_CHECK(msgpack_pack_str_body(pk, seed, PRG_SEED_LENGTH));
P_CHECKC(msgpack_pack_str(pk, PRG_SEED_LENGTH));
P_CHECKC(msgpack_pack_str_body(pk, seed, PRG_SEED_LENGTH));
cleanup:
return rv;
@ -236,7 +236,7 @@ serial_read_prg_seed(msgpack_unpacker* upk, PrioPRGSeed* seed)
P_CHECKCB(upk != NULL);
P_CHECKCB(seed != NULL);
UP_CHECK(msgpack_unpacker_next(upk, &res))
UP_CHECKC(msgpack_unpacker_next(upk, &res));
msgpack_object obj = res.data;
P_CHECKCB(obj.type == MSGPACK_OBJECT_STR);
@ -283,23 +283,23 @@ serial_write_packet_client(msgpack_packer* pk, const_PrioPacketClient p,
P_CHECKCB(pk != NULL);
P_CHECKCB(p != NULL);
P_CHECK(msgpack_pack_str(pk, cfg->batch_id_len));
P_CHECK(msgpack_pack_str_body(pk, cfg->batch_id, cfg->batch_id_len));
P_CHECKC(msgpack_pack_str(pk, cfg->batch_id_len));
P_CHECKC(msgpack_pack_str_body(pk, cfg->batch_id, cfg->batch_id_len));
P_CHECK(serial_write_beaver_triple(pk, p->triple));
P_CHECKC(serial_write_beaver_triple(pk, p->triple));
P_CHECK(serial_write_mp_int(pk, &p->f0_share));
P_CHECK(serial_write_mp_int(pk, &p->g0_share));
P_CHECK(serial_write_mp_int(pk, &p->h0_share));
P_CHECKC(serial_write_mp_int(pk, &p->f0_share));
P_CHECKC(serial_write_mp_int(pk, &p->g0_share));
P_CHECKC(serial_write_mp_int(pk, &p->h0_share));
P_CHECK(msgpack_pack_int(pk, p->for_server));
P_CHECKC(msgpack_pack_int(pk, p->for_server));
switch (p->for_server) {
case PRIO_SERVER_A:
P_CHECK(serial_write_server_a_data(pk, &p->shares.A));
P_CHECKC(serial_write_server_a_data(pk, &p->shares.A));
break;
case PRIO_SERVER_B:
P_CHECK(serial_write_server_b_data(pk, &p->shares.B));
P_CHECKC(serial_write_server_b_data(pk, &p->shares.B));
break;
default:
return SECFailure;
@ -317,7 +317,7 @@ serial_read_server_id(msgpack_unpacker* upk, PrioServerId* s)
P_CHECKCB(s != NULL);
int serv;
P_CHECK(serial_read_int(upk, &serv));
P_CHECKC(serial_read_int(upk, &serv));
P_CHECKCB(serv == PRIO_SERVER_A || serv == PRIO_SERVER_B);
*s = serv;
@ -337,7 +337,7 @@ serial_read_packet_client(msgpack_unpacker* upk, PrioPacketClient p,
P_CHECKCB(upk != NULL);
P_CHECKCB(p != NULL);
UP_CHECK(msgpack_unpacker_next(upk, &res))
UP_CHECKC(msgpack_unpacker_next(upk, &res));
msgpack_object obj = res.data;
P_CHECKCB(obj.type == MSGPACK_OBJECT_STR);
@ -346,23 +346,24 @@ serial_read_packet_client(msgpack_unpacker* upk, PrioPacketClient p,
P_CHECKCB(s.size == cfg->batch_id_len);
P_CHECKCB(!memcmp(s.ptr, (char*)cfg->batch_id, cfg->batch_id_len));
P_CHECK(serial_read_beaver_triple(upk, p->triple, &cfg->modulus));
P_CHECKC(serial_read_beaver_triple(upk, p->triple, &cfg->modulus));
P_CHECK(serial_read_mp_int(upk, &p->f0_share, &cfg->modulus));
P_CHECK(serial_read_mp_int(upk, &p->g0_share, &cfg->modulus));
P_CHECK(serial_read_mp_int(upk, &p->h0_share, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->f0_share, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->g0_share, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->h0_share, &cfg->modulus));
P_CHECK(serial_read_server_id(upk, &p->for_server));
P_CHECKC(serial_read_server_id(upk, &p->for_server));
switch (p->for_server) {
case PRIO_SERVER_A:
P_CHECK(serial_read_server_a_data(upk, &p->shares.A, cfg));
P_CHECKC(serial_read_server_a_data(upk, &p->shares.A, cfg));
break;
case PRIO_SERVER_B:
P_CHECK(serial_read_server_b_data(upk, &p->shares.B));
P_CHECKC(serial_read_server_b_data(upk, &p->shares.B));
break;
default:
return SECFailure;
rv = SECFailure;
goto cleanup;
}
cleanup:
@ -377,8 +378,8 @@ PrioPacketVerify1_write(const_PrioPacketVerify1 p, msgpack_packer* pk)
P_CHECKCB(pk != NULL);
P_CHECKCB(p != NULL);
P_CHECK(serial_write_mp_int(pk, &p->share_d));
P_CHECK(serial_write_mp_int(pk, &p->share_e));
P_CHECKC(serial_write_mp_int(pk, &p->share_d));
P_CHECKC(serial_write_mp_int(pk, &p->share_e));
cleanup:
return rv;
@ -392,8 +393,8 @@ PrioPacketVerify1_read(PrioPacketVerify1 p, msgpack_unpacker* upk,
P_CHECKCB(upk != NULL);
P_CHECKCB(p != NULL);
P_CHECK(serial_read_mp_int(upk, &p->share_d, &cfg->modulus));
P_CHECK(serial_read_mp_int(upk, &p->share_e, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->share_d, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->share_e, &cfg->modulus));
cleanup:
return rv;
@ -406,7 +407,7 @@ PrioPacketVerify2_write(const_PrioPacketVerify2 p, msgpack_packer* pk)
P_CHECKCB(pk != NULL);
P_CHECKCB(p != NULL);
P_CHECK(serial_write_mp_int(pk, &p->share_out));
P_CHECKC(serial_write_mp_int(pk, &p->share_out));
cleanup:
return rv;
@ -420,7 +421,7 @@ PrioPacketVerify2_read(PrioPacketVerify2 p, msgpack_unpacker* upk,
P_CHECKCB(upk != NULL);
P_CHECKCB(p != NULL);
P_CHECK(serial_read_mp_int(upk, &p->share_out, &cfg->modulus));
P_CHECKC(serial_read_mp_int(upk, &p->share_out, &cfg->modulus));
cleanup:
return rv;
@ -432,8 +433,8 @@ PrioTotalShare_write(const_PrioTotalShare t, msgpack_packer* pk)
SECStatus rv = SECSuccess;
P_CHECKCB(t != NULL);
P_CHECKCB(pk != NULL);
P_CHECK(msgpack_pack_int(pk, t->idx));
P_CHECK(serial_write_mp_array(pk, t->data_shares));
P_CHECKC(msgpack_pack_int(pk, t->idx));
P_CHECKC(serial_write_mp_array(pk, t->data_shares));
cleanup:
return rv;
@ -446,8 +447,8 @@ PrioTotalShare_read(PrioTotalShare t, msgpack_unpacker* upk,
SECStatus rv = SECSuccess;
P_CHECKCB(t != NULL);
P_CHECKCB(upk != NULL);
P_CHECK(serial_read_server_id(upk, &t->idx));
P_CHECK(serial_read_mp_array(upk, t->data_shares, cfg->num_data_fields,
P_CHECKC(serial_read_server_id(upk, &t->idx));
P_CHECKC(serial_read_mp_array(upk, t->data_shares, cfg->num_data_fields,
&cfg->modulus));
cleanup:

Просмотреть файл

@ -58,12 +58,15 @@
return SECFailure; \
} while (0);
// Check a msgpack object unpacked correctly
#define UP_CHECK(s) \
// Check a msgpack object unpacked correctly. If
// not, jump to the cleanup label.
#define UP_CHECKC(s) \
do { \
int r = (s); \
if (r != MSGPACK_UNPACK_SUCCESS && r != MSGPACK_UNPACK_EXTRA_BYTES) \
return SECFailure; \
if (r != MSGPACK_UNPACK_SUCCESS && r != MSGPACK_UNPACK_EXTRA_BYTES) { \
rv = SECFailure; \
goto cleanup; \
} \
} while (0);
// Check an MPI library call. If it fails, set the return code and jump