View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005090 | unreal | ircd | public | 2018-04-23 08:47 | 2018-09-09 17:21 |
| Reporter | syzop | Assigned To | syzop | ||
| Priority | normal | Severity | minor | Reproducibility | sometimes |
| Status | resolved | Resolution | fixed | ||
| Target Version | 4.0.18 | Fixed in Version | 4.0.18 | ||
| Summary | 0005090: memory leak via dead_link() | ||||
| Description | ==00:06:03:28.652 26593== 25 bytes in 1 blocks are definitely lost in loss record 559 of 1,646 ==00:06:03:28.652 26593== at 0x4C27BB5: malloc (vg_replace_malloc.c:299) ==00:06:03:28.652 26593== by 0x6476989: strdup (strdup.c:42) ==00:06:03:28.652 26593== by 0x16B9B6: dead_link (send.c:87) ==00:06:03:28.652 26593== by 0x16BB26: send_queued (send.c:142) ==00:06:03:28.652 26593== by 0x16C1BE: sendbufto_one (send.c:282) ==00:06:03:28.652 26593== by 0x16F88F: vsendto_prefix_one (send.c:1207) ==00:06:03:28.652 26593== by 0x16F961: sendto_prefix_one (send.c:1224) ==00:06:03:28.652 26593== by 0x1705FF: sendto_message_one (send.c:1411) ==00:06:03:28.652 26593== by 0x81C3AF6: m_message (m_message.c:419) ==00:06:03:28.652 26593== by 0x81C3CBC: m_private (m_message.c:453) ==00:06:03:28.652 26593== by 0x13A007: parse (parse.c:454) ==00:06:03:28.652 26593== by 0x138A73: dopacket (packet.c:65) ==00:06:03:28.652 26593== ==00:06:03:28.652 26593== 25 bytes in 1 blocks are definitely lost in loss record 560 of 1,646 ==00:06:03:28.652 26593== at 0x4C27BB5: malloc (vg_replace_malloc.c:299) ==00:06:03:28.652 26593== by 0x6476989: strdup (strdup.c:42) ==00:06:03:28.652 26593== by 0x16B9B6: dead_link (send.c:87) ==00:06:03:28.652 26593== by 0x16BB26: send_queued (send.c:142) ==00:06:03:28.652 26593== by 0x16C1BE: sendbufto_one (send.c:282) ==00:06:03:28.652 26593== by 0x16BD7A: vsendto_one (send.c:175) ==00:06:03:28.652 26593== by 0x16BD07: sendto_one (send.c:168) ==00:06:03:28.652 26593== by 0x169BB3: hunt_server (s_user.c:253) ==00:06:03:28.652 26593== by 0x85D2CC8: m_motd (m_motd.c:70) ==00:06:03:28.652 26593== by 0x13A007: parse (parse.c:454) ==00:06:03:28.652 26593== by 0x138A73: dopacket (packet.c:65) ==00:06:03:28.652 26593== by 0x1232F6: parse_client_queued (s_bsd.c:1235) ==00:06:03:28.652 26593== ==00:06:03:28.652 26593== 25 bytes in 1 blocks are definitely lost in loss record 561 of 1,646 ==00:06:03:28.652 26593== at 0x4C27BB5: malloc (vg_replace_malloc.c:299) ==00:06:03:28.652 26593== by 0x6476989: strdup (strdup.c:42) ==00:06:03:28.652 26593== by 0x16B9B6: dead_link (send.c:87) ==00:06:03:28.652 26593== by 0x16BB26: send_queued (send.c:142) ==00:06:03:28.652 26593== by 0x16C1BE: sendbufto_one (send.c:282) ==00:06:03:28.652 26593== by 0x16BD7A: vsendto_one (send.c:175) ==00:06:03:28.652 26593== by 0x16BD07: sendto_one (send.c:168) ==00:06:03:28.652 26593== by 0x16237A: m_dalinfo (s_serv.c:352) ==00:06:03:28.652 26593== by 0x13A007: parse (parse.c:454) ==00:06:03:28.652 26593== by 0x138A73: dopacket (packet.c:65) ==00:06:03:28.652 26593== by 0x1232F6: parse_client_queued (s_bsd.c:1235) ==00:06:03:28.652 26593== by 0x1233BE: process_packet (s_bsd.c:1247) ==00:06:03:28.652 26593== ==00:06:03:28.652 26593== 25 bytes in 1 blocks are definitely lost in loss record 562 of 1,646 ==00:06:03:28.652 26593== at 0x4C27BB5: malloc (vg_replace_malloc.c:299) ==00:06:03:28.652 26593== by 0x6476989: strdup (strdup.c:42) ==00:06:03:28.652 26593== by 0x16B9B6: dead_link (send.c:87) ==00:06:03:28.652 26593== by 0x16BB26: send_queued (send.c:142) ==00:06:03:28.652 26593== by 0x16C1BE: sendbufto_one (send.c:282) ==00:06:03:28.652 26593== by 0x16BD7A: vsendto_one (send.c:175) ==00:06:03:28.652 26593== by 0x16BD07: sendto_one (send.c:168) ==00:06:03:28.652 26593== by 0x6DA5D69: m_admin (m_admin.c:83) ==00:06:03:28.652 26593== by 0x13A007: parse (parse.c:454) ==00:06:03:28.652 26593== by 0x138A73: dopacket (packet.c:65) ==00:06:03:28.652 26593== by 0x1232F6: parse_client_queued (s_bsd.c:1235) ==00:06:03:28.652 26593== by 0x1233BE: process_packet (s_bsd.c:1247) ==00:06:03:28.652 26593== | ||||
| Additional Information | NOTE: this bug was already present 2 days ago, before my mass cleanup ;) | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
So: either it isn't freed via some exit_client function or the pointer is overwritten. Will check later... |
|
|
Seems to me that the latter is not true (not overwritten). There are two places where error_str is set.. in dead_link() and in fatal_ssl_error() and both check for a dead socket and return -1 if the socket is already marked as such (in which case no new strdup() will happen and there will be no overwrite) As for not being freed: free_client frees sptr->local->error_str with the only condition that sptr->local is not NULL (duh) The free_client() function is called from remove_client_from_list() which is called from exit_one_client() unconditionally for everyone. Anyway, sounds like it should be reproducible, so better try that... |
|
|
In check_deadsockets there is this line: cptr->flags &= ~FLAGS_DEADSOCKET; /* CPR. So we send the error. */ which makes sense, but if sending the error results in another error (not too uncommon) then this check in dead_link is skipped: if ((to->flags & FLAGS_DEADSOCKET) && to->local->error_str) return -1; /* already pending to be closed */ and we overwrite error_str: to->local->error_str = strdup(notice); |
|
|
I think the appropriate (and most conservative) fix would be: if (to->flags & FLAGS_DEADSOCKET) return -1; /* already pending to be closed */ to->flags |= FLAGS_DEADSOCKET; if (to->local->error_str) return -1; /* don't overwrite & don't send multiple times */ |
|
|
declassify |
|
|
Fixed on June 11: https://github.com/unrealircd/unrealircd/commit/21af7689c043e25705059a3c0f972f0a9438623d Which went into 4.0.18. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-04-23 08:47 | syzop | New Issue | |
| 2018-04-23 08:47 | syzop | Note Added: 0020105 | |
| 2018-04-23 08:47 | syzop | Status | new => confirmed |
| 2018-04-28 19:55 | syzop | Note Added: 0020106 | |
| 2018-05-01 11:19 | syzop | Note Added: 0020107 | |
| 2018-05-01 11:26 | syzop | Note Added: 0020108 | |
| 2018-05-01 11:26 | syzop | View Status | public => private |
| 2018-09-09 17:19 | syzop | View Status | private => public |
| 2018-09-09 17:19 | syzop | Note Added: 0020305 | |
| 2018-09-09 17:21 | syzop | Assigned To | => syzop |
| 2018-09-09 17:21 | syzop | Status | confirmed => resolved |
| 2018-09-09 17:21 | syzop | Resolution | open => fixed |
| 2018-09-09 17:21 | syzop | Fixed in Version | => 4.0.18 |
| 2018-09-09 17:21 | syzop | Note Added: 0020306 |
