On 8/22/25 12:38 PM, Grisha Levit wrote:
On Wed, Aug 20, 2025 at 5:31 PM Chet Ramey <chet.ra...@case.edu> wrote:On 8/20/25 3:36 PM, Chet Ramey wrote:================================================================= ==1078506==ERROR: AddressSanitizer: heap-use-after-free on address 0x7c315f7e217c at pc 0x55bc1d03b545 bp 0x7ffdbdb3c920 sp 0x7ffdbdb3c910 WRITE of size 4 at 0x7c315f7e217c thread T0 #0 0x55bc1d03b544 in _rl_search_getchar /home/arch/works/bash/lib/ readline/isearch.c:322This is probably the same issue Grisha identified initially: handling SIGINT in rl_read_key ends up invalidating the search context, and then rl_read_key returns -1, which _rl_search_getchar tries to assign to cxt->c_lastc. But I'd like to be able to reproduce it.The real issue is that readline, when reading from its standard input, which is the keyboard in the vast majority of situations, does not believe that read(2) can both succeed (return value > 0) *and* get a signal before it returns to user mode without setting errno. You simply cannot type that fast, since read returns after reading one character. You either need to inject it into the input artificially or hit the process with SIGINT from another process with exquisitely precise timing. I'll have to think about what to do about that.So AFAICT, the case where everything works fine is: upon a SIGINT in rl_getc, the RL_CHECK_SIGNALS() invocation is followed by a call to *rl_signal_event_hook which, in bash, ends up calling throw_to_top_level.
It does, but not for the reason you assume. If the read gets interrupted, and returns -1, everything works correctly. If the read returns 1, but also gets a signal before returning to user mode, as I describe above, then you have problems, regardless of whether or not the application has a signal event hook. You have to choose one to handle, and the fix there is to prioritize handling the signal instead of returning the character.
But most uses of RL_CHECK_SIGNALS() are _not_ followed by a call to *rl_signal_event_hook, so I don't really understand how that can work: if a SIGINT has been received, cleanup code will be executed that leaves whatever readline was doing in a state that it cannot continue from.The real `decider' is the application's SIGINT (or other signal) handler.
RL_CHECK_SIGNALS resets the signal disposition and resends SIGINT to the current process.
Would it work to call *rl_signal_event_hook in more (or all?) places where RL_CHECK_SIGNALS() is used?
No. That's not what the signal event hook is for:
-- Variable: rl_hook_func_t * rl_signal_event_hook
If non-zero, this is the address of a function to call if a read
system call is interrupted by a signal when Readline is reading
terminal input.
and readline should never call it unless it actually received a signal.
The idea is that the application's signal handler gets to decide what to
do. In most cases, it will longjmp back to some processing loop or exit.
The signal event handler provides responsiveness in the case where the
application simply sets a flag in a signal handler (like bash or gdb,
for instance) and doesn't want to wait for additional character input
before handling it.
Now, if the application's signal handler returns after an interrupted read,
and there isn't any signal hook, readline has to decide what to do: should
it continue from where it left off, as the application appears to indicate,
or should it abort the current operation? There are some states where
readline has to abort, since it's already deallocated resources. Those
states are already identified, but the code only calls _rl_abort_internal()
when in callback mode.
Adding it to rl_read_key seems to resolve the OP's reported issue:
diff --git a/lib/readline/input.c b/lib/readline/input.c
index 3383edb6..6885e76d 100644
--- a/lib/readline/input.c
+++ b/lib/readline/input.c
@@ -827,6 +827,8 @@ rl_read_key (void)
c = (*rl_getc_function) (rl_instream);
/* fprintf(stderr, "rl_read_key: calling RL_CHECK_SIGNALS:
_rl_caught_signal = %d\r\n", _rl_caught_signal); */
RL_CHECK_SIGNALS ();
+ if (rl_signal_event_hook)
+ (*rl_signal_event_hook) ();
}
}
This is the wrong place to handle it, no pun intended. In fact, we
probably shouldn't check signals here unless rl_getc_function != rl_getc,
since rl_getc already checks for signals itself. The fix is to make the
code that calls _rl_abort_internal there not dependent on callback mode.
I'll push that change soon; I haven't been able to make bash crash with it,
and it fixes applications that don't have a signal event hook, too.
Chet
--
``The lyf so short, the craft so long to lerne.'' - Chaucer
``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU c...@case.edu http://tiswww.cwru.edu/~chet/
OpenPGP_signature.asc
Description: OpenPGP digital signature
