init_ws_for_output: use workspace_move_to_output

This fixes a crash produced with the following config:
    # i3 config file (v4)
    workspace 1 output $screen1
    workspace 2 output $screen2

    exec --no-startup-id "i3-msg workspace 1, open && i3-msg workspace 2 && xrandr --output $screen2 --off && xrandr --output $screen1 --auto --output $screen2 --auto --right-of $screen1 "

Which results in:
ERROR: AddressSanitizer: heap-use-after-free on address …
READ of size 8 at 0x614000001f48 thread T0
    #0 0x5563df6e73a8 in init_ws_for_output i3/src/randr.c:468
    #1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    #2 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

… is located 264 bytes inside of 448-byte region …
freed by thread T0 here:
    #1 0x5563df634b0a in con_free i3/src/con.c:96
    #2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
    #3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
    #4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
    #5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    #6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

Which is similar to #3228, #3248.
This commit is contained in:
Orestis Floros 2018-09-11 19:11:05 +03:00
parent 83327abae4
commit 4d21f4cfc2
No known key found for this signature in database
GPG Key ID: E9AD9F32E401E38F
2 changed files with 16 additions and 42 deletions

View File

@ -442,47 +442,13 @@ void init_ws_for_output(Output *output, Con *content) {
continue; continue;
} }
/* if so, move it over */ DLOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
LOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n",
workspace->name, workspace_out->name, output_primary_name(output)); workspace->name, workspace_out->name, output_primary_name(output));
/* Need to copy output's rect since content is not yet rendered. We
/* if the workspace is currently visible on that output, we need to * can't call render_con here because render_output only proceeds if a
* switch to a different workspace - otherwise the output would end up * workspace exists. */
* with no active workspace */ content->rect = output->con->rect;
bool visible = workspace_is_visible(workspace); workspace_move_to_output(workspace, output);
Con *previous = NULL;
if (visible && (previous = TAILQ_NEXT(workspace, focused))) {
LOG("Switching to previously used workspace \"%s\" on output \"%s\"\n",
previous->name, workspace_out->name);
workspace_show(previous);
}
/* Render the output on which the workspace was to get correct Rects.
* Then, we need to work with the "content" container, since we cannot
* be sure that the workspace itself was rendered at all (in case its
* invisible, it wont be rendered). */
render_con(workspace_out, false);
Con *ws_out_content = output_get_content(workspace_out);
Con *floating_con;
TAILQ_FOREACH(floating_con, &(workspace->floating_head), floating_windows)
/* NB: We use output->con here because content is not yet rendered,
* so it has a rect of {0, 0, 0, 0}. */
floating_fix_coordinates(floating_con, &(ws_out_content->rect), &(output->con->rect));
con_detach(workspace);
con_attach(workspace, content, false);
/* In case the workspace we just moved was visible but there was no
* other workspace to switch to, we need to initialize the source
* output as well */
if (visible && previous == NULL) {
LOG("There is no workspace left on \"%s\", re-initializing\n",
workspace_out->name);
init_ws_for_output(get_output_by_name(workspace_out->name, true),
output_get_content(workspace_out));
DLOG("Done re-initializing, continuing with \"%s\"\n", output_primary_name(output));
}
} }
/* if a workspace exists, we are done now */ /* if a workspace exists, we are done now */

View File

@ -966,7 +966,11 @@ bool workspace_move_to_output(Con *ws, Output *output) {
LOG("got output %p with content %p\n", output, content); LOG("got output %p with content %p\n", output, content);
Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head)); Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head));
LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name); if (previously_visible_ws) {
DLOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name);
} else {
DLOG("No previously visible workspace on output.\n");
}
bool workspace_was_visible = workspace_is_visible(ws); bool workspace_was_visible = workspace_is_visible(ws);
if (con_num_children(ws->parent) == 1) { if (con_num_children(ws->parent) == 1) {
@ -1024,6 +1028,10 @@ bool workspace_move_to_output(Con *ws, Output *output) {
workspace_show(ws); workspace_show(ws);
} }
if (!previously_visible_ws) {
return true;
}
/* NB: We cannot simply work with previously_visible_ws since it might /* NB: We cannot simply work with previously_visible_ws since it might
* have been cleaned up by workspace_show() already, depending on the * have been cleaned up by workspace_show() already, depending on the
* focus order/number of other workspaces on the output. * focus order/number of other workspaces on the output.