From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id B2D473858D28 for ; Wed, 24 May 2023 18:14:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B2D473858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684952039; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=QLlmWiXk25C2h21cHxlYD23BU4FO8FymQi0RWX/1NLU=; b=HEiGUlvF/ohAXBd7mm22yO1kS+1f4mbneMsMrSMoCFuQx1FgIe3yG5UAyDQQGWnPkw2m+h 2vQ/AKcMNK5TWgRBRmQp70Foy9SwfnJnAtkgZFWmwHT207lVvX26ZDciP6/UMGaW1DZ3bV x4tZm7qI2d8aYYYE2YFO2y8Oia/Wmbo= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-423-VjQ7z5JeOMax2cm3SC_95w-1; Wed, 24 May 2023 14:13:58 -0400 X-MC-Unique: VjQ7z5JeOMax2cm3SC_95w-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-3f5ec8aac77so8278065e9.3 for ; Wed, 24 May 2023 11:13:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684952037; x=1687544037; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QLlmWiXk25C2h21cHxlYD23BU4FO8FymQi0RWX/1NLU=; b=QVI7ofjx574PZZt07ihyedA0+fFVSPMWTMjf0YDoRzED7JCFXU0WqoJiqxq/9nP0gh DcAZMUCCQCaZZbi6eVvKlSON8zqSZ/V6+wfGPNyJZyhf9g8xDe6aKHmbGtCqWBziQrPw M9FN9VXlEeHLE787QhwU0KE/vrWsY66qKygKjOsRMVedHFAsbAw3g0v7VzMUbGn/tbhM J2IO0F8OHpzSo2FFCl7yJInWDshguYlpuUA/eUNf7vSHP3I1YoiWDXEF743N+F/6Qvmi InRatriGE39/v5Db2fUKGxnVCwCCL8eEaVXQDhM/FcXQ6TyME0qAz9+i3W22PfJN4irs +LFw== X-Gm-Message-State: AC+VfDxrQGnq2Ta5IfDayRTWU4Y/flx+2VzZEdT8wYKoXbYjIqmsZ40l f4glKdtnuKMqa8WgGoZeJgM8yQU3BY9nG6sm1gWBP234B8NjeYqZS0AGhbxAMvd8VPAvCTFGOCy BLX7pTC43iEkKBt+FXUV7wA== X-Received: by 2002:a7b:c84a:0:b0:3f5:fb98:729e with SMTP id c10-20020a7bc84a000000b003f5fb98729emr558471wml.22.1684952037491; Wed, 24 May 2023 11:13:57 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7pQOaGDzRA7kKT0WuXhBwyaLj2osQUDdlhTdTwAWi9gUL5SsWTBgnBJEpMxHJzIhwXU9oTqQ== X-Received: by 2002:a7b:c84a:0:b0:3f5:fb98:729e with SMTP id c10-20020a7bc84a000000b003f5fb98729emr558457wml.22.1684952037086; Wed, 24 May 2023 11:13:57 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id 2-20020a05600c028200b003f42cc3262asm3047233wmk.34.2023.05.24.11.13.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 May 2023 11:13:56 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org, bug-readline@gnu.org Subject: Re: [PATCH] [readline] Fix double free in _rl_scxt_dispose In-Reply-To: <20230523160431.28769-1-tdevries@suse.de> References: <20230523160431.28769-1-tdevries@suse.de> Date: Wed, 24 May 2023 19:13:55 +0100 Message-ID: <87lehd61i4.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom de Vries writes: > 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. > > PR tui/30056 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30056 > --- > readline/readline/isearch.c | 3 +++ > readline/readline/mbutil.c | 2 +- > 2 files changed, 4 insertions(+), 1 deletion(-) Have you posted this to the readline list? I think it would be best if we at least posted patches like this upstream before we merge them. Thanks, Andrew > > 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); > > 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 > { > > base-commit: 9196be90bd9572bd09fad63d7e0b2fa199738b90 > -- > 2.35.3