From eb97ef08b71a8042d1812aea3ebefa2de4146dfb Mon Sep 17 00:00:00 2001 From: Charlie Gordon Date: Sun, 7 May 2017 20:32:30 +0000 Subject: [PATCH] shell: fix spurious crash bug - add check_mode_data() to verify mode data pointer validity - avoid double free in qe_free_mode_data() and shell_pid_cb() --- qe.c | 26 +++++++++++++++++++++++++- qe.h | 1 + shell.c | 12 +++++++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/qe.c b/qe.c index adbba62..3764841 100644 --- a/qe.c +++ b/qe.c @@ -2146,6 +2146,26 @@ void *qe_get_window_mode_data(EditState *e, ModeDef *m, int status) return NULL; } +void *check_mode_data(void **pp) { + QEmacsState *qs = &qe_state; + QEModeData *md = *pp; + EditBuffer *b; + EditState *e; + + for (b = qs->first_buffer; b != NULL; b = b->next) { + QEModeData **mdp; + for (mdp = &b->mode_data_list; *mdp; mdp = &(*mdp)->next) { + if (*mdp == md) + return md; + } + } + for (e = qs->first_window; e != NULL; e = e->next_window) { + if (e->mode_data == md) + return md; + } + return NULL; +} + int qe_free_mode_data(QEModeData *md) { int rc = -1; @@ -2173,7 +2193,10 @@ int qe_free_mode_data(QEModeData *md) rc = 0; } } - qe_free(&md); + if (rc == 0) { + /* mode data was found, OK to free */ + qe_free(&md); + } return rc; } @@ -5773,6 +5796,7 @@ void edit_close(EditState **sp) /* save current state for later window reattachment */ switch_to_buffer(s, NULL); edit_detach(s); + /* closing the window mode should have freed it already */ qe_free_mode_data(s->mode_data); qe_free(&s->prompt); qe_free(&s->line_shadow); diff --git a/qe.h b/qe.h index f2b04b3..3c1a6a4 100644 --- a/qe.h +++ b/qe.h @@ -1572,6 +1572,7 @@ QEModeData *qe_create_buffer_mode_data(EditBuffer *b, ModeDef *m); void *qe_get_buffer_mode_data(EditBuffer *b, ModeDef *m, EditState *e); QEModeData *qe_create_window_mode_data(EditState *s, ModeDef *m); void *qe_get_window_mode_data(EditState *e, ModeDef *m, int status); +void *check_mode_data(void **pp); int qe_free_mode_data(QEModeData *md); /* from tty.c */ diff --git a/shell.c b/shell.c index 808700d..35441ea 100644 --- a/shell.c +++ b/shell.c @@ -1627,6 +1627,7 @@ static void qe_term_emulate(ShellState *s, int c) for (i = 0; i < s->nb_params; i++) { switch (s->params[i]) { case 1: /* Application Cursor Keys (DECCKM). */ + case 3: /* Set Number of Columns to 132 (DECCOLM) */ case 4: /* Smooth (Slow) Scroll (DECSCLM). */ break; case 5: /* Reverse Video (DECSCNM) */ @@ -1712,6 +1713,7 @@ static void qe_term_emulate(ShellState *s, int c) for (i = 0; i < s->nb_params; i++) { switch (s->params[i]) { case 1: /* Normal Cursor Keys (DECCKM). */ + case 3: /* Set Number of Columns to 80 (DECCOLM) */ case 4: /* Jump (Fast) Scroll (DECSCLM). */ break; case 5: /* Normal Video (DECSCNM). */ @@ -1942,12 +1944,17 @@ static void shell_mode_free(EditBuffer *b, void *state) static void shell_pid_cb(void *opaque, int status) { - ShellState *s = opaque; + ShellState *s; EditBuffer *b; QEmacsState *qs; EditState *e; char buf[1024]; + /* Extra check in case mode data was freed already. + * It is not completely fool proof as the same address might have + * been reallocated for the same purpose, quite unlikely. + */ + s = check_mode_data(&opaque); if (!s || s->base.mode != &shell_mode) return; @@ -2007,6 +2014,8 @@ static void shell_pid_cb(void *opaque, int status) qe_set_next_mode(e, 0, 0); } if (!(s->shell_flags & SF_INTERACTIVE)) { + /* Must Unlink the shell data to avoid potential crash */ + //shell_mode_free(b, s); // called by qe_free_mode_data qe_free_mode_data(&s->base); } edit_display(qs); @@ -2082,6 +2091,7 @@ EditBuffer *new_shell_buffer(EditBuffer *b0, EditState *e, return NULL; } + /* XXX: ShellState life cycle is bogus */ set_read_handler(s->pty_fd, shell_read_cb, s); set_pid_handler(s->pid, shell_pid_cb, s); return b;