Fix an use-after-free bug (#2522)
Fix the issue #2421 (https://github.com/i3/i3/issues/2421). floating_enable() invokes tree_close_internal() to free con->parent. After con->parent is freed in tree_close_internal() but before con->parent is reassigned by the caller, con->parent may be dereferenced and causes i3 crash. The backtrace below is an example. The already-freed pointer is dereferenced again through the pointer "focused" in x_push_changes(). Reassign con->parent before calling tree_close_internal() to fix this use-after-free bug. 0x0000000000416372 in con_get_workspace (con=0x7ab9c0) at ../i3/src/con.c:375 0x0000000000416103 in con_has_managed_window (con=0x7ab9c0) at ../i3/src/con.c:266 0x000000000042b413 in x_push_changes (con=0x78d190) at ../i3/src/x.c:1132 0x0000000l0004533e8 in tree_render () at ../i3/src/tree.c:504 0x0000000000452b4f in tree_close_internal (con=0x7b67c0, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false) ../i3/src/tree.c:314 0x00000000004196f0 in con_on_remove_child (con=0x7b67c0) at ../i3/src/con.c:1801 0x0000000000452eb7 in tree_close_internal (con=0x783840, kill_window=DONT_KILL_WINDOW, dont_kill_parent=false, force_set_focus=false) ../i3/src/tree.c:364 0x0000000000431516 in floating_enable (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:183 0x0000000000431eed in toggle_floating_mode (con=0x7ab9c0, automatic=false) at ../i3/src/floating.c:379 0x0000000000420d92 in cmd_floating (current_match=0x679a20 , cmd_output=0x679aa0 , floating_mode=0x7ab8c0 "toggle") ../i3/src/commands.c:1088 0x000000000043e5ae in GENERATED_call (call_identifier=60, result=0x679aa0 ) at include/GENERATED_command_call.h:486 0x000000000043ee19 in next_state (token=0x675d70 ) at ../i3/src/commands_parser.c:187 0x000000000043f2fb in parse_command (input=0x7b4fe0 "floating toggle", gen=0x0) at ../i3/src/commands_parser.c:308 0x00000000004125f8 in run_binding (bind=0x784260, con=0x0) at ../i3/src/bindings.c:792 0x000000000042bace in handle_key_press (event=0x7a01a0) at ../i3/src/key_press.c:33 0x000000000044e6aa in handle_event (type=2, event=0x7a01a0) at ../i3/src/handlers.c:1420 0x0000000000439533 in xcb_check_cb (loop=0x7ffff532f8e0, w=0x68c140, revents=32768) at ../i3/src/main.c:133 0x00007ffff5125d73 in ev_invoke_pending () from /usr/lib/x86_64-linux-gnu/libev.so.4 0x00007ffff51293de in ev_run () from /usr/lib/x86_64-linux-gnu/libev.so.4 0x0000000000439418 in ev_loop (loop=0x7ffff532f8e0, flags=0) at /usr/include/ev.h:835 0x000000000043d51d in main (argc=3, argv=0x7fffffffe0a8) at ../i3/src/main.c:913
This commit is contained in:
parent
841118e5f8
commit
faa9915abc
|
@ -178,7 +178,10 @@ void floating_enable(Con *con, bool automatic) {
|
||||||
if ((con->parent->type == CT_CON || con->parent->type == CT_FLOATING_CON) &&
|
if ((con->parent->type == CT_CON || con->parent->type == CT_FLOATING_CON) &&
|
||||||
con_num_children(con->parent) == 0) {
|
con_num_children(con->parent) == 0) {
|
||||||
DLOG("Old container empty after setting this child to floating, closing\n");
|
DLOG("Old container empty after setting this child to floating, closing\n");
|
||||||
tree_close_internal(con->parent, DONT_KILL_WINDOW, false, false);
|
Con *parent = con->parent;
|
||||||
|
/* clear the pointer before calling tree_close_internal in which the memory is freed */
|
||||||
|
con->parent = NULL;
|
||||||
|
tree_close_internal(parent, DONT_KILL_WINDOW, false, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
char *name;
|
char *name;
|
||||||
|
@ -315,6 +318,7 @@ void floating_disable(Con *con, bool automatic) {
|
||||||
const bool set_focus = (con == focused);
|
const bool set_focus = (con == focused);
|
||||||
|
|
||||||
Con *ws = con_get_workspace(con);
|
Con *ws = con_get_workspace(con);
|
||||||
|
Con *parent = con->parent;
|
||||||
|
|
||||||
/* 1: detach from parent container */
|
/* 1: detach from parent container */
|
||||||
TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
|
TAILQ_REMOVE(&(con->parent->nodes_head), con, nodes);
|
||||||
|
@ -323,7 +327,9 @@ void floating_disable(Con *con, bool automatic) {
|
||||||
/* 2: kill parent container */
|
/* 2: kill parent container */
|
||||||
TAILQ_REMOVE(&(con->parent->parent->floating_head), con->parent, floating_windows);
|
TAILQ_REMOVE(&(con->parent->parent->floating_head), con->parent, floating_windows);
|
||||||
TAILQ_REMOVE(&(con->parent->parent->focus_head), con->parent, focused);
|
TAILQ_REMOVE(&(con->parent->parent->focus_head), con->parent, focused);
|
||||||
tree_close_internal(con->parent, DONT_KILL_WINDOW, true, false);
|
/* clear the pointer before calling tree_close_internal in which the memory is freed */
|
||||||
|
con->parent = NULL;
|
||||||
|
tree_close_internal(parent, DONT_KILL_WINDOW, true, false);
|
||||||
|
|
||||||
/* 3: re-attach to the parent of the currently focused con on the workspace
|
/* 3: re-attach to the parent of the currently focused con on the workspace
|
||||||
* this floating con was on */
|
* this floating con was on */
|
||||||
|
|
|
@ -846,9 +846,11 @@ void randr_disable_output(Output *output) {
|
||||||
}
|
}
|
||||||
|
|
||||||
DLOG("destroying disappearing con %p\n", output->con);
|
DLOG("destroying disappearing con %p\n", output->con);
|
||||||
tree_close_internal(output->con, DONT_KILL_WINDOW, true, false);
|
Con *con = output->con;
|
||||||
DLOG("Done. Should be fine now\n");
|
/* clear the pointer before calling tree_close_internal in which the memory is freed */
|
||||||
output->con = NULL;
|
output->con = NULL;
|
||||||
|
tree_close_internal(con, DONT_KILL_WINDOW, true, false);
|
||||||
|
DLOG("Done. Should be fine now\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
output->to_be_disabled = false;
|
output->to_be_disabled = false;
|
||||||
|
|
Loading…
Reference in New Issue