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:
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
