Skip to content

Commit 4fe2359

Browse files
Fix memory leak with oneofs and PB_ENABLE_MALLOC (#615)
Nanopb would leak memory when all of the following conditions were true: - PB_ENABLE_MALLOC is defined at the compile time - Message definitions contains an oneof field, the oneof contains a static submessage, and the static submessage contains a pointer field. - Data being decoded contains two values for the submessage. The logic in pb_release_union_field would detect that the same submessage occurs twice, and wouldn't release it because keeping the old values is necessary to match the C++ library behavior regarding message merges. But then decode_static_field() would go to memset() the whole submessage to zero, because it unconditionally assumed it to be uninitialized memory. This would normally happen when the contents of the union field is switched to a different oneof item, instead of merging with the same one. This commit changes it so that the field is memset() only when `which_field` contains a different tag.
1 parent d9d5dfd commit 4fe2359

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

pb_decode.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,14 +464,17 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t
464464
}
465465

466466
case PB_HTYPE_ONEOF:
467-
*(pb_size_t*)iter->pSize = iter->pos->tag;
468-
if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE)
467+
if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE &&
468+
*(pb_size_t*)iter->pSize != iter->pos->tag)
469469
{
470470
/* We memset to zero so that any callbacks are set to NULL.
471-
* Then set any default values. */
471+
* This is because the callbacks might otherwise have values
472+
* from some other union field. */
472473
memset(iter->pData, 0, iter->pos->data_size);
473474
pb_message_set_to_defaults((const pb_field_t*)iter->pos->ptr, iter->pData);
474475
}
476+
*(pb_size_t*)iter->pSize = iter->pos->tag;
477+
475478
return func(stream, iter->pos, iter->pData);
476479

477480
default:

0 commit comments

Comments
 (0)