Great, committed, thanks
On Wed, Feb 08, 2017 at 10:46:43AM -0600, attila wrote: > > Nicholas Marriott <[email protected]> writes: > > > Yes, it is no longer OK for window_copy_pagedown to reset copy mode, it > > needs to let the caller do it (because there is stuff to free, and it > > can only know it needs to free it by dereferencing the mode data). I > > changed the rest and missed this one. > > > > Try this please: > > This fixes it. > > Thanks for looking into it. > > Pax, -A > > > Index: window-copy.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/tmux/window-copy.c,v > > retrieving revision 1.165 > > diff -u -p -r1.165 window-copy.c > > --- window-copy.c 3 Feb 2017 11:57:28 -0000 1.165 > > +++ window-copy.c 8 Feb 2017 11:17:06 -0000 > > @@ -29,7 +29,7 @@ static void window_copy_command(struct w > > struct session *, struct args *, struct mouse_event *); > > static struct screen *window_copy_init(struct window_pane *); > > static void window_copy_free(struct window_pane *); > > -static void window_copy_pagedown(struct window_pane *, int); > > +static int window_copy_pagedown(struct window_pane *, int); > > static void window_copy_next_paragraph(struct window_pane *); > > static void window_copy_previous_paragraph(struct window_pane *); > > static void window_copy_resize(struct window_pane *, u_int, u_int); > > @@ -380,7 +380,7 @@ window_copy_pageup(struct window_pane *w > > window_copy_redraw_screen(wp); > > } > > > > -static void > > +static int > > window_copy_pagedown(struct window_pane *wp, int half_page) > > { > > struct window_copy_mode_data *data = wp->modedata; > > @@ -420,13 +420,11 @@ window_copy_pagedown(struct window_pane > > window_copy_cursor_end_of_line(wp); > > } > > > > - if (data->scroll_exit && data->oy == 0) { > > - window_pane_reset_mode(wp); > > - return; > > - } > > - > > + if (data->scroll_exit && data->oy == 0) > > + return (1); > > window_copy_update_selection(wp, 1); > > window_copy_redraw_screen(wp); > > + return (0); > > } > > > > static void > > @@ -621,8 +619,12 @@ window_copy_command(struct window_pane * > > if (strcmp(command, "end-of-line") == 0) > > window_copy_cursor_end_of_line(wp); > > if (strcmp(command, "halfpage-down") == 0) { > > - for (; np != 0; np--) > > - window_copy_pagedown(wp, 1); > > + for (; np != 0; np--) { > > + if (window_copy_pagedown(wp, 1)) { > > + cancel = 1; > > + break; > > + } > > + } > > } > > if (strcmp(command, "halfpage-up") == 0) { > > for (; np != 0; np--) > > @@ -715,8 +717,12 @@ window_copy_command(struct window_pane * > > window_copy_other_end(wp); > > } > > if (strcmp(command, "page-down") == 0) { > > - for (; np != 0; np--) > > - window_copy_pagedown(wp, 0); > > + for (; np != 0; np--) { > > + if (window_copy_pagedown(wp, 0)) { > > + cancel = 1; > > + break; > > + } > > + } > > } > > if (strcmp(command, "page-up") == 0) { > > for (; np != 0; np--) > > > > > > > > > > On Wed, Feb 08, 2017 at 06:56:55AM +0100, Otto Moerbeek wrote: > >> On Tue, Feb 07, 2017 at 10:59:00AM -0600, attila wrote: > >> > >> > > >> > [email protected] writes: > >> > > >> > >>Synopsis: tmux can be made to dump core when a certain bind-key > >> > >>option is present > >> > >>Category: system > >> > >>Environment: > >> > > System : OpenBSD 6.0 > >> > > Details : OpenBSD 6.0-current (GENERIC.MP) #158: Mon Jan 30 > >> > > 19:30:12 MST 2017 > >> > > > >> > > [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP > >> > > > >> > > Architecture: OpenBSD.amd64 > >> > > Machine : amd64 > >> > >>Description: > >> > > > >> > > When in a tmux session hitting the command prefix key + PgUp and PgDn > >> > > immediately afterwards causes a core dump if a certain bind-key option > >> > > is present. The backtrace is not useful (/etc/malloc.conf -> CFGJU). > >> > > > >> > > This bind-key is not necessary as far as I can tell, since the default > >> > > behavior of recent tmux seems to be what that bind-key intends to do > >> > > (enter copy mode when C-b PageUp is pressed). I don't know how I had > >> > > it in my ~/.tmux.conf, but commenting it out stops the core dump. > >> > > > >> > >>How-To-Repeat: > >> > > (1) tmux with the default configuration (mv away your ~/.tmux.conf > >> > > or touch nonexistent_file && tmux -f nonexistent_file) > >> > > (2) Hit C-b : bind-key PageUp copy-mode -eu > >> > > (3) Hit C-b PageUp PageDown > >> > > >> > Here is a backtrace from this snap: > >> > > >> > OpenBSD 6.0-current (GENERIC.MP) #163: Sun Feb 5 13:55:12 MST 2017 > >> > [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP > >> > > >> > > >> > (gdb) bt > >> > #0 0x0000075cf82507aa in thrkill () at <stdin>:2 > >> > #1 0x0000075cf828f749 in *_libc_abort () at > >> > /usr/src/lib/libc/stdlib/abort.c:52 > >> > #2 0x0000075cf8230012 in wrterror (d=Variable "d" is not available. > >> > ) at /usr/src/lib/libc/stdlib/malloc.c:303 > >> > #3 0x0000075cf8231284 in ofree (argpool=0x75c37135ca0, > >> > p=0xdfdfdfdfdfdfdfdf) > >> > at /usr/src/lib/libc/stdlib/malloc.c:1374 > >> > #4 0x0000075cf82314eb in free (ptr=0xdfdfdfdfdfdfdfdf) at > >> > /usr/src/lib/libc/stdlib/malloc.c:1407 > >> > #5 0x0000075a03942208 in tty_cmd_clearcharacter () from /usr/bin/tmux > >> > #6 0x0000075a039453c8 in tty_cmd_clearcharacter () from /usr/bin/tmux > >> > #7 0x0000075a0390cb02 in ?? () from /usr/bin/tmux > >> > #8 0x0000075a03914ab2 in ?? () from /usr/bin/tmux > >> > #9 0x0000075a03932cf0 in control_callback () from /usr/bin/tmux > >> > #10 0x0000075a03929908 in control_callback () from /usr/bin/tmux > >> > #11 0x0000075a03932c8a in control_callback () from /usr/bin/tmux > >> > #12 0x0000075a039038f4 in ?? () from /usr/bin/tmux > >> > #13 0x0000075a03939bdf in control_callback () from /usr/bin/tmux > >> > #14 0x0000075a03900f6e in ?? () from /usr/bin/tmux > >> > #15 0x0000000000000000 in ?? () > >> > >> Hi, > >> > >> I took a look because I wanted to know if this is falloout from recent > >> malloc changes. > >> > >> Compiled with -O0 -g I get a slightly different stacktrace, see > >> below. This does look like a genuine user-after-free. Probably nicm@ > >> can spot the bug easier than me so I included him in the To:. > >> > >> -Otto > >> > >> #4 0x00001f49066e6c4b in free (ptr=0xdfdfdfdfdfdfdfdf) > >> at /usr/src/lib/libc/stdlib/malloc.c:1407 > >> #5 0x00001f46dad5c3f5 in window_copy_clear_marks (wp=0x1f49501b1800) > >> at /usr/src/usr.bin/tmux/window-copy.c:1212 > >> #6 0x00001f46dad5b7b9 in window_copy_command (wp=0x1f49501b1800, > >> c=0x1f49d6363800, s=0x1f4902252200, args=0x1f4904f00ca0, m=0x0) > >> at /usr/src/usr.bin/tmux/window-copy.c:920 > >> #7 0x00001f46dad0fe98 in cmd_send_keys_exec (self=0x1f49b7a42780, > >> item=0x1f4904f03a00) at /usr/src/usr.bin/tmux/cmd-send-keys.c:90 > >> #8 0x00001f46dad1a995 in cmdq_fire_command (item=0x1f4904f03a00) > >> at /usr/src/usr.bin/tmux/cmd-queue.c:203 > >> #9 0x00001f46dad1ae7d in cmdq_next (c=0x1f49d6363800) > >> at /usr/src/usr.bin/tmux/cmd-queue.c:325 > >> #10 0x00001f46dad454a8 in server_loop () at > >> /usr/src/usr.bin/tmux/server.c:198 > >> #11 0x00001f46dad392b1 in proc_loop (tp=0x1f48edc832c0, > >> loopcb=0x1f46dad45467 <server_loop>) at > >> /usr/src/usr.bin/tmux/proc.c:220 > >> #12 0x00001f46dad4543a in server_start (base=0x1f4924ac5c00, lockfd=8, > >> lockfile=0x1f49270bc120 "10.1.1.4 55392 10.1.1.3 22") > >> at /usr/src/usr.bin/tmux/server.c:182 > >> #13 0x00001f46dad0463b in client_connect (base=0x1f4924ac5c00, > >> path=0x1f49594a5460 "/tmp/tmux-1000/default", start_server=1) > >> at /usr/src/usr.bin/tmux/client.c:160 > >> #14 0x00001f46dad0498c in client_main (base=0x1f4924ac5c00, argc=0, > >> argv=0x7f7ffffc5ff0, flags=0, shellcmd=0x0) > >> at /usr/src/usr.bin/tmux/client.c:268 > >> #15 0x00001f46dad4ef8b in main (argc=0, argv=0x7f7ffffc5ff0) > >> at /usr/src/usr.bin/tmux/tmux.c:349 > -- > https://haqistan.net/~attila | attila@{stalphonsos.com,haqistan.net} > pgp: 0x62A729CF | C2CE 2487 03AC 4C2F 101D 09C1 4068 D5D5 62A7 29CF
