alibaba / xquic

XQUIC Library released by Alibaba is a cross-platform implementation of QUIC and HTTP/3 protocol.
Apache License 2.0
1.7k stars 327 forks source link

[Bug]: memory leak: xqc_process_crypto_frame: stream_frame #264

Closed Luffbee closed 1 year ago

Luffbee commented 1 year ago

What happened?

The memory allocate for stream_frame may leak.

Steps To Reproduce

valgrind it in high loss environment.

Relevant log output

No response

Kulsk commented 1 year ago

do you have the leak stack print ? and is test_client/test_server used ?

Luffbee commented 1 year ago

do you have the leak stack print ? and is test_client/test_server used ?

==14794== 92 (56 direct, 36 indirect) bytes in 1 blocks are definitely lost in loss record 801 of 1,655
==14794==    at 0x4C2C089: calloc (vg_replace_malloc.c:762)
==14794==    by 0x4EBDCEC: xqc_calloc (xqc_malloc.h:51)
==14794==    by 0x4EBDCEC: xqc_process_crypto_frame (xqc_frame.c:921)
==14794==    by 0x4EBFC17: xqc_process_frames (xqc_frame.c:650)
==14794==    by 0x4EBA586: xqc_packet_decrypt_single (xqc_packet.c:183)
==14794==    by 0x4EBA75B: xqc_packet_process_single (xqc_packet.c:226)
==14794==    by 0x4EA5F6C: xqc_conn_process_packet (xqc_conn.c:2702)
==14794==    by 0x4EA2040: xqc_engine_packet_process (xqc_engine.c:1004)

Found it with my custom application.

Kulsk commented 1 year ago

thanks Luffbee, recently I also met some memory leak problems in our software, will try to fix ASAP.

do you have the leak stack print ? and is test_client/test_server used ?

==14794== 92 (56 direct, 36 indirect) bytes in 1 blocks are definitely lost in loss record 801 of 1,655
==14794==    at 0x4C2C089: calloc (vg_replace_malloc.c:762)
==14794==    by 0x4EBDCEC: xqc_calloc (xqc_malloc.h:51)
==14794==    by 0x4EBDCEC: xqc_process_crypto_frame (xqc_frame.c:921)
==14794==    by 0x4EBFC17: xqc_process_frames (xqc_frame.c:650)
==14794==    by 0x4EBA586: xqc_packet_decrypt_single (xqc_packet.c:183)
==14794==    by 0x4EBA75B: xqc_packet_process_single (xqc_packet.c:226)
==14794==    by 0x4EA5F6C: xqc_conn_process_packet (xqc_conn.c:2702)
==14794==    by 0x4EA2040: xqc_engine_packet_process (xqc_engine.c:1004)

Found it with my custom application.

Luffbee commented 1 year ago

do you have the leak stack print ? and is test_client/test_server used ?

==14794== 92 (56 direct, 36 indirect) bytes in 1 blocks are definitely lost in loss record 801 of 1,655
==14794==    at 0x4C2C089: calloc (vg_replace_malloc.c:762)
==14794==    by 0x4EBDCEC: xqc_calloc (xqc_malloc.h:51)
==14794==    by 0x4EBDCEC: xqc_process_crypto_frame (xqc_frame.c:921)
==14794==    by 0x4EBFC17: xqc_process_frames (xqc_frame.c:650)
==14794==    by 0x4EBA586: xqc_packet_decrypt_single (xqc_packet.c:183)
==14794==    by 0x4EBA75B: xqc_packet_process_single (xqc_packet.c:226)
==14794==    by 0x4EA5F6C: xqc_conn_process_packet (xqc_conn.c:2702)
==14794==    by 0x4EA2040: xqc_engine_packet_process (xqc_engine.c:1004)

Found it with my custom application.

I modified some part of xquic, so the line number may not right for you. And I never touched crypto stream part, so the problem should not caused by my modification.

Kulsk commented 1 year ago

could you upload the log ? it might help to confirm the leak scenario

Kulsk commented 1 year ago

I found it might happen if stream_frame->data is created while error occurs during subsequent xqc_parse_crypto_frame or xqc_create_crypto_stream, the stream_frame->data will not be freed.