* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
@ 2023-01-27 3:04 ` sam at gentoo dot org
2023-01-28 20:00 ` ssbssa at sourceware dot org
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: sam at gentoo dot org @ 2023-01-27 3:04 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
Sam James <sam at gentoo dot org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |sam at gentoo dot org
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
2023-01-27 3:04 ` [Bug tui/30056] " sam at gentoo dot org
@ 2023-01-28 20:00 ` ssbssa at sourceware dot org
2023-05-23 8:38 ` vries at gcc dot gnu.org
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: ssbssa at sourceware dot org @ 2023-01-28 20:00 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
Hannes Domani <ssbssa at sourceware dot org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ssbssa at sourceware dot org
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
2023-01-27 3:04 ` [Bug tui/30056] " sam at gentoo dot org
2023-01-28 20:00 ` ssbssa at sourceware dot org
@ 2023-05-23 8:38 ` vries at gcc dot gnu.org
2023-05-23 9:53 ` vries at gcc dot gnu.org
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 8:38 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
Tom de Vries <vries at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
CC| |vries at gcc dot gnu.org
Last reconfirmed| |2023-05-23
Ever confirmed|0 |1
--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
This fixes it for me:
...
diff --git a/readline/readline/callback.c b/readline/readline/callback.c
index 93f23d97bc2..e2a97f7cee7 100644
--- a/readline/readline/callback.c
+++ b/readline/readline/callback.c
@@ -163,7 +163,7 @@ rl_callback_read_char (void)
RL_CHECK_SIGNALS ();
if (RL_ISSTATE (RL_STATE_ISEARCH))
{
- eof = _rl_isearch_callback (_rl_iscxt);
+ eof = _rl_isearch_callback (&_rl_iscxt);
if (eof == 0 && (RL_ISSTATE (RL_STATE_ISEARCH) == 0) && RL_ISSTATE
(RL_STATE_INPUTPENDING))
rl_callback_read_char ();
diff --git a/readline/readline/isearch.c b/readline/readline/isearch.c
index 080ba3cbb9c..9f9b72d3b8e 100644
--- a/readline/readline/isearch.c
+++ b/readline/readline/isearch.c
@@ -877,14 +877,17 @@ rl_search_history (int direction, int invoking_key)
If _rl_isearch_dispatch finishes searching, this function is responsible
for turning off RL_STATE_ISEARCH, which it does using _rl_isearch_cleanup.
*/
int
-_rl_isearch_callback (_rl_search_cxt *cxt)
+_rl_isearch_callback (_rl_search_cxt **cxt)
{
int c, r;
- c = _rl_search_getchar (cxt);
+ c = _rl_search_getchar (*cxt);
+ if (*cxt == 0)
+ return 1;
/* We might want to handle EOF here */
- r = _rl_isearch_dispatch (cxt, cxt->lastc);
-
- return (r <= 0) ? _rl_isearch_cleanup (cxt, r) : 0;
+ r = _rl_isearch_dispatch (*cxt, (*cxt)->lastc);
+ if (*cxt == 0)
+ return 1;
+ return (r <= 0) ? _rl_isearch_cleanup (*cxt, r) : 0;
}
#endif
diff --git a/readline/readline/rlprivate.h b/readline/readline/rlprivate.h
index 02838ae21ae..a549ec62607 100644
--- a/readline/readline/rlprivate.h
+++ b/readline/readline/rlprivate.h
@@ -306,7 +306,7 @@ extern _rl_search_cxt *_rl_scxt_alloc PARAMS((int, int));
extern void _rl_scxt_dispose PARAMS((_rl_search_cxt *, int));
extern int _rl_isearch_dispatch PARAMS((_rl_search_cxt *, int));
-extern int _rl_isearch_callback PARAMS((_rl_search_cxt *));
+extern int _rl_isearch_callback PARAMS((_rl_search_cxt **));
extern int _rl_isearch_cleanup PARAMS((_rl_search_cxt *, int));
extern int _rl_search_getchar PARAMS((_rl_search_cxt *));
...
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (2 preceding siblings ...)
2023-05-23 8:38 ` vries at gcc dot gnu.org
@ 2023-05-23 9:53 ` vries at gcc dot gnu.org
2023-05-23 10:29 ` vries at gcc dot gnu.org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 9:53 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
The simplest way of fixing this is:
...
diff --git a/readline/readline/isearch.c b/readline/readline/isearch.c
index 080ba3cbb9c..407c8b61543 100644
--- a/readline/readline/isearch.c
+++ b/readline/readline/isearch.c
@@ -882,7 +882,8 @@ _rl_isearch_callback (_rl_search_cxt *cxt)
int c, r;
c = _rl_search_getchar (cxt);
- /* We might want to handle EOF here */
+ if (c == EOF)
+ return 1;
r = _rl_isearch_dispatch (cxt, cxt->lastc);
return (r <= 0) ? _rl_isearch_cleanup (cxt, r) : 0;
...
but the question is if that sometimes skips _rl_isearch_cleanup while it
shouldn't.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (3 preceding siblings ...)
2023-05-23 9:53 ` vries at gcc dot gnu.org
@ 2023-05-23 10:29 ` vries at gcc dot gnu.org
2023-05-23 11:50 ` vries at gcc dot gnu.org
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 10:29 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
This looks ok:
...
diff --git a/readline/readline/isearch.c b/readline/readline/isearch.c
index 080ba3cbb9c..941078f790e 100644
--- a/readline/readline/isearch.c
+++ b/readline/readline/isearch.c
@@ -882,6 +882,9 @@ _rl_isearch_callback (_rl_search_cxt *cxt)
int c, r;
c = _rl_search_getchar (cxt);
+ if (!RL_ISSTATE (RL_STATE_ISEARCH))
+ return 1;
+
/* We might want to handle EOF here */
r = _rl_isearch_dispatch (cxt, cxt->lastc);
...
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (4 preceding siblings ...)
2023-05-23 10:29 ` vries at gcc dot gnu.org
@ 2023-05-23 11:50 ` vries at gcc dot gnu.org
2023-05-23 15:06 ` vries at gcc dot gnu.org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 11:50 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
Alternatively, we can regard this as a multibyte problem:
...
diff --git a/readline/readline/mbutil.c b/readline/readline/mbutil.c
index dc62b4cc24d..7da3ff17bb5 100644
--- a/readline/readline/mbutil.c
+++ b/readline/readline/mbutil.c
@@ -363,7 +363,7 @@ _rl_get_char_len (char *src, mbstate_t *ps)
/* Look at no more than MB_CUR_MAX characters */
l = (size_t)strlen (src);
- if (_rl_utf8locale && l > 0 && UTF8_SINGLEBYTE(*src))
+ if (_rl_utf8locale && l >= 0 && UTF8_SINGLEBYTE(*src))
tmp = (*src != 0) ? 1 : 0;
else
{
...
For "right arrow", tui_getc returns 0 and expects readline to treat it as a
nop.
However, readline treats it as the first part of a multi-byte character. The
patch above fixes this, and allows it to be recognized as UTF8_SINGLEBYTE.
Consequently, we have a different call stack when ^C hit us, and the problem
doesn't trigger.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (5 preceding siblings ...)
2023-05-23 11:50 ` vries at gcc dot gnu.org
@ 2023-05-23 15:06 ` vries at gcc dot gnu.org
2023-05-23 16:06 ` vries at gcc dot gnu.org
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 15:06 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 14904
--> https://sourceware.org/bugzilla/attachment.cgi?id=14904&action=edit
Tentative patch
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (6 preceding siblings ...)
2023-05-23 15:06 ` vries at gcc dot gnu.org
@ 2023-05-23 16:06 ` vries at gcc dot gnu.org
2023-05-24 14:11 ` vries at gcc dot gnu.org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-23 16:06 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #5)
> Created attachment 14904 [details]
> Tentative patch
Submitted: https://sourceware.org/pipermail/gdb-patches/2023-May/199785.html
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (7 preceding siblings ...)
2023-05-23 16:06 ` vries at gcc dot gnu.org
@ 2023-05-24 14:11 ` vries at gcc dot gnu.org
2023-05-28 8:17 ` cvs-commit at gcc dot gnu.org
2023-05-28 8:27 ` vries at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-24 14:11 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
Tom de Vries <vries at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |raphael at pdev dot de
--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
*** Bug 27900 has been marked as a duplicate of this bug. ***
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (8 preceding siblings ...)
2023-05-24 14:11 ` vries at gcc dot gnu.org
@ 2023-05-28 8:17 ` cvs-commit at gcc dot gnu.org
2023-05-28 8:27 ` vries at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-28 8:17 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=85f4cf41a852b5983ca436615a019315f6dc7301
commit 85f4cf41a852b5983ca436615a019315f6dc7301
Author: Tom de Vries <tdevries@suse.de>
Date: Sun May 28 10:17:57 2023 +0200
[readline] Fix double free in _rl_scxt_dispose
Consider the following scenario. We start gdb in TUI mode:
...
$ gdb -q -tui
...
and type ^R which gives us the reverse-isearch prompt in the cmd window:
...
(reverse-i-search)`':
...
and then type "foo", right-arrow-key, and ^C.
In TUI mode, gdb uses a custom rl_getc_function tui_getc.
When pressing the right-arrow-key, tui_getc:
- attempts to scroll the TUI src window, without any effect, and
- returns 0.
The intention of returning 0 is mentioned here in tui_dispatch_ctrl_char:
...
/* We intercepted the control character, so return 0 (which readline
will interpret as a no-op). */
return 0;
...
However, after this 0 is returned by the rl_read_key () call in
_rl_search_getchar, _rl_read_mbstring is called, which incorrectly
interprets
0 as the first part of an utf-8 multibyte char, and tries to read the next
char.
In this state, the ^C takes effect and we run into a double free because
_rl_isearch_cleanup is called twice.
Both these issues need fixing independently, though after fixing the first
we
no longer trigger the second.
The first issue is caused by the subtle difference between:
- a char array containing 0 chars, which is zero-terminated, and
- a char array containing 1 char, which is zero.
In mbrtowc terms, this is the difference between:
...
mbrtowc (&wc, "", 0, &ps);
...
which returns -2, and:
...
mbrtowc (&wc, "", 1, &ps);
...
which returns 0.
Note that _rl_read_mbstring calls _rl_get_char_len without passing it an
explicit length parameter, and consequently it cannot distinguish between
the
two, and defaults to the "0 chars" choice.
Note that the same problem doesn't exist in _rl_read_mbchar.
Fix this by defaulting to the "1 char" choice in _rl_get_char_len:
...
- if (_rl_utf8locale && l > 0 && UTF8_SINGLEBYTE(*src))
+ if (_rl_utf8locale && l >= 0 && UTF8_SINGLEBYTE(*src))
...
The second problem happens when the call to _rl_search_getchar in
_rl_isearch_callback returns. At that point _rl_isearch_cleanup has
already
been called from the signal handler, but we proceed regardless, using a cxt
pointer that has been freed.
Fix this by checking for "RL_ISSTATE (RL_STATE_ISEARCH)" after the call to
_rl_search_getchar:
...
c = _rl_search_getchar (cxt);
+ if (!RL_ISSTATE (RL_STATE_ISEARCH))
+ return 1;
...
Tested on x86_64-linux.
Approved-By: Chet Ramey <chet.ramey@case.edu>
PR tui/30056
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30056
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bug tui/30056] double free when using reverse-search for a previous command and Ctrl-C
2023-01-26 17:47 [Bug tui/30056] New: double free when using reverse-search for a previous command and Ctrl-C etesta at undo dot io
` (9 preceding siblings ...)
2023-05-28 8:17 ` cvs-commit at gcc dot gnu.org
@ 2023-05-28 8:27 ` vries at gcc dot gnu.org
10 siblings, 0 replies; 12+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-28 8:27 UTC (permalink / raw)
To: gdb-prs
https://sourceware.org/bugzilla/show_bug.cgi?id=30056
Tom de Vries <vries at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
Target Milestone|--- |14.1
--- Comment #9 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed by commit in the in-repo copy of readline.
The problem is still there for system readlines that miss this patch, but
AFAICT there's nothing we can do about this in gdb.
That is, maybe we could fix half of the problem by not returning '\0' to
readline, but instead call tui_getc_1 in a loop until it returns something not
'\0'. But the other half of the problem would still exist and cause the same
problem.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply [flat|nested] 12+ messages in thread