patch for bug in window_copy_append_selection

[J Raynor]

There's a bug in window_copy_append_selection that causes tmux to free
memory that shouldn't be freed.

To reproduce the problem:

* Set mode-keys to vi so you can use A to append to a buffer
* Enter copy mode and copy some text
* Use lsb to note that the buffer names look normal
* Enter copy mode again, select some text, and hit A
* Use lsb again, and you should see the top buffer's name has been truncated


The problem is this line:

bufname = pb->name

Later in the function, paste_set is called, passing in bufname.
Eventually, this gets to paste_free_name, which frees the bufname,
paste_set then tries to create a new pb with the name that was passed
in, but that memory was just freed.

I've attached a patch that will fix this.

[Nicholas Marriott]

Hi

Would this fix it instead?

Index: paste.c
===================================================================
RCS file: /cvs/src/usr.bin/tmux/paste.c,v
retrieving revision 1.26
diff -u -p -r1.26 paste.c
--- paste.c 5 Nov 2014 23:25:02 -0000 1.26
+++ paste.c 31 Mar 2015 17:50:54 -0000
@@ -247,9 +247,6 @@ paste_set(char *data, size_t size, const
return (-1);
}

- pb = paste_get_name(name);
- if (pb != NULL)
- paste_free_name(name);

pb = xmalloc(sizeof *pb);

@@ -260,6 +257,9 @@ paste_set(char *data, size_t size, const

pb->automatic = 0;
pb->order = paste_next_order++;
+
+ if (paste_get_name(name) != NULL)
+ paste_free_name(name);

RB_INSERT(paste_name_tree, &paste_by_name, pb);
RB_INSERT(paste_time_tree, &paste_by_time, pb);

Post by J Raynor
There's a bug in window_copy_append_selection that causes tmux to free
memory that shouldn't be freed.
* Set mode-keys to vi so you can use A to append to a buffer
* Enter copy mode and copy some text
* Use lsb to note that the buffer names look normal
* Enter copy mode again, select some text, and hit A
* Use lsb again, and you should see the top buffer's name has been truncated
bufname = pb->name
Later in the function, paste_set is called, passing in bufname.
Eventually, this gets to paste_free_name, which frees the bufname,
paste_set then tries to create a new pb with the name that was passed
in, but that memory was just freed.
I've attached a patch that will fix this.
diff --git a/window-copy.c b/window-copy.c
index 223df88..17a0034 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -1540,7 +1540,7 @@ window_copy_copy_selection(struct window_pane *wp, const char *bufname)
void
window_copy_append_selection(struct window_pane *wp, const char *bufname)
{
- char *buf;
+ char *buf, *tmpbufname;
struct paste_buffer *pb;
size_t len;
struct screen_write_ctx ctx;
@@ -1556,19 +1556,25 @@ window_copy_append_selection(struct window_pane *wp, const char *bufname)
}
if (bufname == NULL || *bufname == '\0') {
+ tmpbufname = NULL;
pb = paste_get_top();
if (pb != NULL)
- bufname = pb->name;
- } else
- pb = paste_get_name(bufname);
+ tmpbufname = xstrdup(pb->name);
+ } else {
+ tmpbufname = xstrdup(bufname);
+ pb = paste_get_name(tmpbufname);
+ }
+
if (pb != NULL) {
buf = xrealloc(buf, len + pb->size);
memmove(buf + pb->size, buf, len);
memcpy(buf, pb->data, pb->size);
len += pb->size;
}
- if (paste_set(buf, len, bufname, NULL) != 0)
+ if (paste_set(buf, len, tmpbufname, NULL) != 0)
free(buf);
+
+ free(tmpbufname);
}
void
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users

[J Raynor]

Yes, this fixes it.

On Tue, Mar 31, 2015 at 12:51 PM, Nicholas Marriott

Post by Nicholas Marriott
Hi
Would this fix it instead?
Index: paste.c
===================================================================
RCS file: /cvs/src/usr.bin/tmux/paste.c,v
retrieving revision 1.26
diff -u -p -r1.26 paste.c
--- paste.c 5 Nov 2014 23:25:02 -0000 1.26
+++ paste.c 31 Mar 2015 17:50:54 -0000
@@ -247,9 +247,6 @@ paste_set(char *data, size_t size, const
return (-1);
}
- pb = paste_get_name(name);
- if (pb != NULL)
- paste_free_name(name);
pb = xmalloc(sizeof *pb);
@@ -260,6 +257,9 @@ paste_set(char *data, size_t size, const
pb->automatic = 0;
pb->order = paste_next_order++;
+
+ if (paste_get_name(name) != NULL)
+ paste_free_name(name);
RB_INSERT(paste_name_tree, &paste_by_name, pb);
RB_INSERT(paste_time_tree, &paste_by_time, pb);

Post by J Raynor
There's a bug in window_copy_append_selection that causes tmux to free
memory that shouldn't be freed.
* Set mode-keys to vi so you can use A to append to a buffer
* Enter copy mode and copy some text
* Use lsb to note that the buffer names look normal
* Enter copy mode again, select some text, and hit A
* Use lsb again, and you should see the top buffer's name has been truncated
bufname = pb->name
Later in the function, paste_set is called, passing in bufname.
Eventually, this gets to paste_free_name, which frees the bufname,
paste_set then tries to create a new pb with the name that was passed
in, but that memory was just freed.
I've attached a patch that will fix this.
diff --git a/window-copy.c b/window-copy.c
index 223df88..17a0034 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -1540,7 +1540,7 @@ window_copy_copy_selection(struct window_pane *wp, const char *bufname)
void
window_copy_append_selection(struct window_pane *wp, const char *bufname)
{
- char *buf;
+ char *buf, *tmpbufname;
struct paste_buffer *pb;
size_t len;
struct screen_write_ctx ctx;
@@ -1556,19 +1556,25 @@ window_copy_append_selection(struct window_pane *wp, const char *bufname)
}
if (bufname == NULL || *bufname == '\0') {
+ tmpbufname = NULL;
pb = paste_get_top();
if (pb != NULL)
- bufname = pb->name;
- } else
- pb = paste_get_name(bufname);
+ tmpbufname = xstrdup(pb->name);
+ } else {
+ tmpbufname = xstrdup(bufname);
+ pb = paste_get_name(tmpbufname);
+ }
+
if (pb != NULL) {
buf = xrealloc(buf, len + pb->size);
memmove(buf + pb->size, buf, len);
memcpy(buf, pb->data, pb->size);
len += pb->size;
}
- if (paste_set(buf, len, bufname, NULL) != 0)
+ if (paste_set(buf, len, tmpbufname, NULL) != 0)
free(buf);
+
+ free(tmpbufname);
}
void
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users

[Nicholas Marriott]

cool, applied, thanks

Post by J Raynor
Yes, this fixes it.
On Tue, Mar 31, 2015 at 12:51 PM, Nicholas Marriott

Post by Nicholas Marriott
Hi
Would this fix it instead?
Index: paste.c
===================================================================
RCS file: /cvs/src/usr.bin/tmux/paste.c,v
retrieving revision 1.26
diff -u -p -r1.26 paste.c
--- paste.c 5 Nov 2014 23:25:02 -0000 1.26
+++ paste.c 31 Mar 2015 17:50:54 -0000
@@ -247,9 +247,6 @@ paste_set(char *data, size_t size, const
return (-1);
}
- pb = paste_get_name(name);
- if (pb != NULL)
- paste_free_name(name);
pb = xmalloc(sizeof *pb);
@@ -260,6 +257,9 @@ paste_set(char *data, size_t size, const
pb->automatic = 0;
pb->order = paste_next_order++;
+
+ if (paste_get_name(name) != NULL)
+ paste_free_name(name);
RB_INSERT(paste_name_tree, &paste_by_name, pb);
RB_INSERT(paste_time_tree, &paste_by_time, pb);

Post by J Raynor
There's a bug in window_copy_append_selection that causes tmux to free
memory that shouldn't be freed.
* Set mode-keys to vi so you can use A to append to a buffer
* Enter copy mode and copy some text
* Use lsb to note that the buffer names look normal
* Enter copy mode again, select some text, and hit A
* Use lsb again, and you should see the top buffer's name has been truncated
bufname = pb->name
Later in the function, paste_set is called, passing in bufname.
Eventually, this gets to paste_free_name, which frees the bufname,
paste_set then tries to create a new pb with the name that was passed
in, but that memory was just freed.
I've attached a patch that will fix this.
diff --git a/window-copy.c b/window-copy.c
index 223df88..17a0034 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -1540,7 +1540,7 @@ window_copy_copy_selection(struct window_pane *wp, const char *bufname)
void
window_copy_append_selection(struct window_pane *wp, const char *bufname)
{
- char *buf;
+ char *buf, *tmpbufname;
struct paste_buffer *pb;
size_t len;
struct screen_write_ctx ctx;
@@ -1556,19 +1556,25 @@ window_copy_append_selection(struct window_pane *wp, const char *bufname)
}
if (bufname == NULL || *bufname == '\0') {
+ tmpbufname = NULL;
pb = paste_get_top();
if (pb != NULL)
- bufname = pb->name;
- } else
- pb = paste_get_name(bufname);
+ tmpbufname = xstrdup(pb->name);
+ } else {
+ tmpbufname = xstrdup(bufname);
+ pb = paste_get_name(tmpbufname);
+ }
+
if (pb != NULL) {
buf = xrealloc(buf, len + pb->size);
memmove(buf + pb->size, buf, len);
memcpy(buf, pb->data, pb->size);
len += pb->size;
}
- if (paste_set(buf, len, bufname, NULL) != 0)
+ if (paste_set(buf, len, tmpbufname, NULL) != 0)
free(buf);
+
+ free(tmpbufname);
}
void
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users