| Summary: | ASTERISK-21466: [patch] [crash] command (sip show peers) crashes Asterisk with ~3500 registered peers | ||||
| Reporter: | Guillaume Knispel (gknispel) | Labels: | |||
| Date Opened: | 2013-04-17 05:29:14 | Date Closed: | 2013-05-01 13:41:38 | ||
| Priority: | Critical | Regression? | No | ||
| Status: | Closed/Complete | Components: | Channels/chan_sip/General | ||
| Versions: | 11.3.0 | Frequency of Occurrence | Constant | ||
| Related Issues: |
| ||||
| Environment: | Linux 32 bits | Attachments: | ( 0) fix_sip_show_peers_stack_overflow_asterisk_11.3.0.patch ( 1) fix_sip_show_peers_stack_overflow_asterisk_11.3.0-v2.patch | ||
| Description: | When there are lots of registered sip peers (around 3500, but it could depend on the IP adresses), the "sip show peers" CLI command immediately crashes Asterisk by stack overflow. (Trying to get the same informations through AMI very probably leads to the same crash.)
This is caused by the use of ast_strdupa() in the main loop of _sip_show_peers() | ||||
| Comments: | By: Guillaume Knispel (gknispel) 2013-04-17 05:32:03.680-0500 I've got a patch ready (that uses ast_strdup() instead of ast_strdupa(), and properly ast_free() the buffers) Waiting for acceptation of my License Agreement to post it. By: Guillaume Knispel (gknispel) 2013-04-17 05:38:30.515-0500 I've tried to find other occurrences of that pattern (strdupa, alloca in a loop) automatically using Coccinelle, but did not succeed (Coccinelle runs forever, maybe a bug, an error in my unsterstanding of its grammar, or maybe Coccinelle just does not like Asterisk source code). So here are some notes about some other functions I found by a (very limited) manual review, that uses strdupa/alloca in a loop and could benefit from being reviewed: chan_sip: static char *_sip_show_peers(int fd, int *total, struct mansession *s, const struct message *m, int argc, const char *argv[]) crash static char *sip_show_settings(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a) localaddr ? static struct sip_peer *build_peer(const char *name, struct ast_variable *v, struct ast_variable *alt, int realtime, int devstate_only) app_voicemail.c: static int imap_retrieve_greeting(const char *dir, const int msgnum, struct ast_vm_user *vmu) static int forward_message(struct ast_channel *chan, char *context, struct vm_state *vms, struct ast_vm_user *sender, char *fmt, int is_new_message, signed char record_gain, int urgent) static int imap_delete_old_greeting (char *dir, struct vm_state *vms) static int actual_load_config(int reload, struct ast_config *cfg, struct ast_config *ucfg) static void vm_change_password(struct ast_vm_user *vmu, const char *newpassword) I insist that this is a very very non exhaustive list. By: Richard Mudgett (rmudgett) 2013-04-17 11:58:56.241-0500 The patch does not check the return value of ast_strdup() for NULL. By: Guillaume Knispel (gknispel) 2013-04-19 10:44:53.634-0500 Ok I'm redoing the patch by extracting the content of the for loop into a separated function instead. By: Guillaume Knispel (gknispel) 2013-04-20 12:07:47.184-0500 New patch attached in fix_sip_show_peers_stack_overflow_asterisk_11.3.0-v2.patch | ||||