Make user removals and traversals thread safe in meetme.

Race conditions present in meetme involving the user list where a lack of
locking has the potential for a user to be removed during a traversal or as in
the case of the reporter after checking if the list is empty could cause a
crash. Fixing this was done by convering the userlist to an ao2 container.

(closes issue #17390)
Reported by: Vince

Review: https://reviewboard.asterisk.org/r/746/


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.4@275773 65c4cc65-6c06-0410-ace0-fbb531ad65f3
1.4
Jeff Peeler 15 years ago
parent cd054ab3b4
commit c66d95c8e5

@ -362,7 +362,7 @@ struct ast_conference {
struct ast_frame *transframe[32];
struct ast_frame *origframe;
struct ast_trans_pvt *transpath[32];
AST_LIST_HEAD_NOLOCK(, ast_conf_user) userlist;
struct ao2_container *usercontainer;
AST_LIST_ENTRY(ast_conference) list;
/* announce_thread related data */
pthread_t announcethread;
@ -752,6 +752,30 @@ static void conf_play(struct ast_channel *chan, struct ast_conference *conf, enu
ast_autoservice_stop(chan);
}
static int user_no_cmp(void *obj, void *arg, int flags)
{
struct ast_conf_user *user = obj;
int *user_no = arg;
if (user->user_no == *user_no) {
return (CMP_MATCH | CMP_STOP);
}
return 0;
}
static int user_max_cmp(void *obj, void *arg, int flags)
{
struct ast_conf_user *user = obj;
int *max_no = arg;
if (user->user_no > *max_no) {
*max_no = user->user_no;
}
return 0;
}
/*!
* \brief Find or create a conference
*
@ -782,8 +806,10 @@ static struct ast_conference *build_conf(char *confno, char *pin, char *pinadmin
goto cnfout;
/* Make a new one */
if (!(cnf = ast_calloc(1, sizeof(*cnf))))
if (!(cnf = ast_calloc(1, sizeof(*cnf))) ||
!(cnf->usercontainer = ao2_container_alloc(1, NULL, user_no_cmp))) {
goto cnfout;
}
ast_mutex_init(&cnf->playlock);
ast_mutex_init(&cnf->listenlock);
@ -939,6 +965,7 @@ static int meetme_cmd(int fd, int argc, char **argv)
strncat(cmdline, argv[3], sizeof(cmdline) - strlen(cmdline) - 1);
}
} else if(strcmp(argv[1], "list") == 0) {
struct ao2_iterator user_iter;
int concise = ( 4 == argc && ( !strcasecmp(argv[3], "concise") ) );
/* List all the users in a conference */
if (AST_LIST_EMPTY(&confs)) {
@ -960,11 +987,12 @@ static int meetme_cmd(int fd, int argc, char **argv)
}
/* Show all the users */
time(&now);
AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
user_iter = ao2_iterator_init(cnf->usercontainer, 0);
while((user = ao2_iterator_next(&user_iter))) {
hr = (now - user->jointime) / 3600;
min = ((now - user->jointime) % 3600) / 60;
sec = (now - user->jointime) % 60;
if ( !concise )
if (!concise) {
ast_cli(fd, "User #: %-2.2d %12.12s %-20.20s Channel: %s %s %s %s %s %02d:%02d:%02d\n",
user->user_no,
S_OR(user->chan->cid.cid_num, "<unknown>"),
@ -974,7 +1002,7 @@ static int meetme_cmd(int fd, int argc, char **argv)
user->userflags & CONFFLAG_MONITOR ? "(Listen only)" : "",
user->adminflags & ADMINFLAG_MUTED ? "(Admin Muted)" : user->adminflags & ADMINFLAG_SELFMUTED ? "(Muted)" : "",
istalking(user->talking), hr, min, sec);
else
} else {
ast_cli(fd, "%d!%s!%s!%s!%s!%s!%s!%d!%02d:%02d:%02d\n",
user->user_no,
S_OR(user->chan->cid.cid_num, ""),
@ -984,8 +1012,10 @@ static int meetme_cmd(int fd, int argc, char **argv)
user->userflags & CONFFLAG_MONITOR ? "1" : "",
user->adminflags & (ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED) ? "1" : "",
user->talking, hr, min, sec);
}
ao2_ref(user, -1);
}
ao2_iterator_destroy(&user_iter);
if ( !concise )
ast_cli(fd,"%d users in that conference.\n",cnf->users);
AST_LIST_UNLOCK(&confs);
@ -1044,15 +1074,21 @@ static char *complete_meetmecmd(const char *line, const char *word, int pos, int
}
if (cnf) {
struct ao2_iterator user_iter;
user_iter = ao2_iterator_init(cnf->usercontainer, 0);
/* Search for the user */
AST_LIST_TRAVERSE(&cnf->userlist, usr, list) {
while((usr = ao2_iterator_next(&user_iter))) {
snprintf(usrno, sizeof(usrno), "%d", usr->user_no);
if (!strncasecmp(word, usrno, len) && ++which > state)
if (!strncasecmp(word, usrno, len) && ++which > state) {
ao2_ref(usr, -1);
break;
}
ao2_ref(usr, -1);
}
ao2_iterator_destroy(&user_iter);
AST_LIST_UNLOCK(&confs);
return usr ? strdup(usrno) : NULL;
}
AST_LIST_UNLOCK(&confs);
return usr ? strdup(usrno) : NULL;
} else if ( strstr(line, "list") && ( 0 == state ) )
return strdup("concise");
}
@ -1302,6 +1338,9 @@ static int conf_free(struct ast_conference *conf)
ast_hangup(conf->chan);
if (conf->fd >= 0)
close(conf->fd);
if (conf->usercontainer) {
ao2_ref(conf->usercontainer, -1);
}
ast_mutex_destroy(&conf->playlock);
ast_mutex_destroy(&conf->listenlock);
@ -1317,13 +1356,19 @@ static void conf_queue_dtmf(const struct ast_conference *conf,
const struct ast_conf_user *sender, struct ast_frame *f)
{
struct ast_conf_user *user;
struct ao2_iterator user_iter;
AST_LIST_TRAVERSE(&conf->userlist, user, list) {
if (user == sender)
user_iter = ao2_iterator_init(conf->usercontainer, 0);
while ((user = ao2_iterator_next(&user_iter))) {
if (user == sender) {
ao2_ref(user, -1);
continue;
}
if (ast_write(user->chan, f) < 0)
ast_log(LOG_WARNING, "Error writing frame to channel %s\n", user->chan->name);
ao2_ref(user, -1);
}
ao2_iterator_destroy(&user_iter);
}
static void sla_queue_event_full(enum sla_event_type type,
@ -1564,8 +1609,9 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
int setusercount = 0;
int confsilence = 0, totalsilence = 0;
if (!(user = ast_calloc(1, sizeof(*user))))
if (!(user = ao2_alloc(sizeof(*user), NULL))) {
return ret;
}
/* Possible timeout waiting for marked user */
if ((confflags & CONFFLAG_WAITMARKED) &&
@ -1630,13 +1676,11 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
}
ast_mutex_lock(&conf->playlock);
if (AST_LIST_EMPTY(&conf->userlist))
user->user_no = 1;
else
user->user_no = AST_LIST_LAST(&conf->userlist)->user_no + 1;
AST_LIST_INSERT_TAIL(&conf->userlist, user, list);
ao2_lock(conf->usercontainer);
ao2_callback(conf->usercontainer, OBJ_NODATA, user_max_cmp, &user->user_no);
user->user_no++;
ao2_link(conf->usercontainer, user);
ao2_unlock(conf->usercontainer);
user->chan = chan;
user->userflags = confflags;
@ -2202,15 +2246,21 @@ static int conf_run(struct ast_channel *chan, struct ast_conference *conf, int c
}
break;
case '3': /* Eject last user */
{
int max_no = 0;
ao2_callback(conf->usercontainer, OBJ_NODATA, user_max_cmp, &max_no);
menu_active = 0;
usr = AST_LIST_LAST(&conf->userlist);
usr = ao2_find(conf->usercontainer, &max_no, 0);
if ((usr->chan->name == chan->name)||(usr->userflags & CONFFLAG_ADMIN)) {
if(!ast_streamfile(chan, "conf-errormenu", chan->language))
ast_waitstream(chan, "");
} else
} else {
usr->adminflags |= ADMINFLAG_KICKME;
}
ao2_ref(user, -1);
ast_stopstream(chan);
break;
}
case '4':
tweak_listen_volume(user, VOL_DOWN);
break;
@ -2470,7 +2520,9 @@ bailoutandtrynormal:
if (dsp)
ast_dsp_free(dsp);
if (user->user_no) { /* Only cleanup users who really joined! */
if (!user->user_no) {
ao2_ref(user, -1);
} else { /* Only cleanup users who really joined! */
now = time(NULL);
hr = (now - user->jointime) / 3600;
min = ((now - user->jointime) % 3600) / 60;
@ -2500,8 +2552,8 @@ bailoutandtrynormal:
if (confflags & CONFFLAG_MARKEDUSER)
conf->markedusers--;
}
/* Remove ourselves from the list */
AST_LIST_REMOVE(&conf->userlist, user, list);
/* Remove ourselves from the container */
ao2_unlink(conf->usercontainer, user);
/* Change any states */
if (!conf->users)
@ -2511,7 +2563,6 @@ bailoutandtrynormal:
snprintf(meetmesecs, sizeof(meetmesecs), "%d", (int) (time(NULL) - user->jointime));
pbx_builtin_setvar_helper(chan, "MEETMESECS", meetmesecs);
}
free(user);
AST_LIST_UNLOCK(&confs);
return ret;
@ -2973,14 +3024,71 @@ static struct ast_conf_user *find_user(struct ast_conference *conf, char *caller
sscanf(callerident, "%30i", &cid);
if (conf && callerident) {
AST_LIST_TRAVERSE(&conf->userlist, user, list) {
if (cid == user->user_no)
return user;
}
user = ao2_find(conf->usercontainer, &cid, 0);
/* reference decremented later in admin_exec */
return user;
}
return NULL;
}
static int user_set_kickme_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
user->adminflags |= ADMINFLAG_KICKME;
return 0;
}
static int user_set_muted_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
if (!(user->userflags & CONFFLAG_ADMIN)) {
user->adminflags |= ADMINFLAG_MUTED;
}
return 0;
}
static int user_set_unmuted_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED);
return 0;
}
static int user_listen_volup_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
tweak_listen_volume(user, VOL_UP);
return 0;
}
static int user_listen_voldown_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
tweak_listen_volume(user, VOL_DOWN);
return 0;
}
static int user_talk_volup_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
tweak_talk_volume(user, VOL_UP);
return 0;
}
static int user_talk_voldown_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
tweak_talk_volume(user, VOL_DOWN);
return 0;
}
static int user_reset_vol_cb(void *obj, void *unused, int flags)
{
struct ast_conf_user *user = obj;
reset_volumes(user);
return 0;
}
/*! \brief The MeetMeadmin application */
/* MeetMeAdmin(confno, command, caller) */
static int admin_exec(struct ast_channel *chan, void *data) {
@ -3026,8 +3134,13 @@ static int admin_exec(struct ast_channel *chan, void *data) {
ast_atomic_fetchadd_int(&cnf->refcount, 1);
if (args.user)
if (args.user) {
user = find_user(cnf, args.user);
if (!user) {
ast_log(LOG_NOTICE, "Specified User not found!\n");
goto usernotfound;
}
}
switch (*args.command) {
case 76: /* L: Lock */
@ -3037,96 +3150,72 @@ static int admin_exec(struct ast_channel *chan, void *data) {
cnf->locked = 0;
break;
case 75: /* K: kick all users */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
user->adminflags |= ADMINFLAG_KICKME;
ao2_callback(cnf->usercontainer, 0, user_set_kickme_cb, NULL);
break;
case 101: /* e: Eject last user*/
user = AST_LIST_LAST(&cnf->userlist);
{
int max_no = 0;
ao2_callback(cnf->usercontainer, OBJ_NODATA, user_max_cmp, &max_no);
user = ao2_find(cnf->usercontainer, &max_no, 0);
if (!(user->userflags & CONFFLAG_ADMIN))
user->adminflags |= ADMINFLAG_KICKME;
else
ast_log(LOG_NOTICE, "Not kicking last user, is an Admin!\n");
ao2_ref(user, -1);
break;
}
case 77: /* M: Mute */
if (user) {
user->adminflags |= ADMINFLAG_MUTED;
} else
ast_log(LOG_NOTICE, "Specified User not found!\n");
user->adminflags |= ADMINFLAG_MUTED;
break;
case 78: /* N: Mute all (non-admin) users */
AST_LIST_TRAVERSE(&cnf->userlist, user, list) {
if (!(user->userflags & CONFFLAG_ADMIN))
user->adminflags |= ADMINFLAG_MUTED;
}
ao2_callback(cnf->usercontainer, 0, user_set_muted_cb, NULL);
break;
case 109: /* m: Unmute */
if (user) {
user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED);
} else
ast_log(LOG_NOTICE, "Specified User not found!\n");
user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED);
break;
case 110: /* n: Unmute all users */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
user->adminflags &= ~(ADMINFLAG_MUTED | ADMINFLAG_SELFMUTED);
ao2_callback(cnf->usercontainer, 0, user_set_unmuted_cb, NULL);
break;
case 107: /* k: Kick user */
if (user)
user->adminflags |= ADMINFLAG_KICKME;
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
user->adminflags |= ADMINFLAG_KICKME;
break;
case 118: /* v: Lower all users listen volume */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
tweak_listen_volume(user, VOL_DOWN);
ao2_callback(cnf->usercontainer, 0, user_listen_voldown_cb, NULL);
break;
case 86: /* V: Raise all users listen volume */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
tweak_listen_volume(user, VOL_UP);
ao2_callback(cnf->usercontainer, 0, user_listen_volup_cb, NULL);
break;
case 115: /* s: Lower all users speaking volume */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
tweak_talk_volume(user, VOL_DOWN);
ao2_callback(cnf->usercontainer, 0, user_talk_voldown_cb, NULL);
break;
case 83: /* S: Raise all users speaking volume */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
tweak_talk_volume(user, VOL_UP);
ao2_callback(cnf->usercontainer, 0, user_talk_volup_cb, NULL);
break;
case 82: /* R: Reset all volume levels */
AST_LIST_TRAVERSE(&cnf->userlist, user, list)
reset_volumes(user);
ao2_callback(cnf->usercontainer, 0, user_reset_vol_cb, NULL);
break;
case 114: /* r: Reset user's volume level */
if (user)
reset_volumes(user);
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
reset_volumes(user);
break;
case 85: /* U: Raise user's listen volume */
if (user)
tweak_listen_volume(user, VOL_UP);
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
tweak_listen_volume(user, VOL_UP);
break;
case 117: /* u: Lower user's listen volume */
if (user)
tweak_listen_volume(user, VOL_DOWN);
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
tweak_listen_volume(user, VOL_DOWN);
break;
case 84: /* T: Raise user's talk volume */
if (user)
tweak_talk_volume(user, VOL_UP);
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
tweak_talk_volume(user, VOL_UP);
break;
case 116: /* t: Lower user's talk volume */
if (user)
tweak_talk_volume(user, VOL_DOWN);
else
ast_log(LOG_NOTICE, "Specified User not found!\n");
tweak_talk_volume(user, VOL_DOWN);
break;
}
if (args.user) {
/* decrement reference from find_user */
ao2_ref(user, -1);
}
usernotfound:
AST_LIST_UNLOCK(&confs);
dispose_conf(cnf);
@ -3174,9 +3263,7 @@ static int meetmemute(struct mansession *s, const struct message *m, int mute)
return 0;
}
AST_LIST_TRAVERSE(&conf->userlist, user, list)
if (user->user_no == userno)
break;
user = ao2_find(conf->usercontainer, &userno, 0);
if (!user) {
AST_LIST_UNLOCK(&confs);
@ -3193,6 +3280,7 @@ static int meetmemute(struct mansession *s, const struct message *m, int mute)
ast_log(LOG_NOTICE, "Requested to %smute conf %s user %d userchan %s uniqueid %s\n", mute ? "" : "un", conf->confno, user->user_no, user->chan->name, user->chan->uniqueid);
ao2_ref(user, -1);
astman_send_ack(s, m, mute ? "User muted" : "User unmuted");
return 0;
}

Loading…
Cancel
Save