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
