Message ID | 20190218124438.GB20127@altlinux.org |
---|---|
State | Superseded |
Headers |
Received: (qmail 114974 invoked by alias); 18 Feb 2019 12:44:42 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 114960 invoked by uid 89); 18 Feb 2019 12:44:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1141 X-HELO: vmicros1.altlinux.org Date: Mon, 18 Feb 2019 15:44:39 +0300 From: "Dmitry V. Levin" <ldv@altlinux.org> To: libc-alpha@sourceware.org Subject: [PATCH] libio: do not cleanup wide buffers of legacy standard files [BZ #24228] Message-ID: <20190218124438.GB20127@altlinux.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline |
Commit Message
Dmitry V. Levin
Feb. 18, 2019, 12:44 p.m. UTC
Commit glibc-2.23~693 (a601b74d31ca086de38441d316a3dee24c866305) introduced a regression: _IO_unbuffer_all() now invokes _IO_wsetb() to free wide buffers of all files, including legacy standard files that are small statically allocated objects that do not have wide buffers. Fix this by skipping _IO_wsetb() invocation for legacy standard files. [BZ #24228] * libio/genops.c (_IO_unbuffer_all) [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)]: Skip _IO_wsetb() invocation for legacy standard files. --- ChangeLog | 7 +++++++ libio/genops.c | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-)
Comments
* Dmitry V. Levin: > Commit glibc-2.23~693 (a601b74d31ca086de38441d316a3dee24c866305) > introduced a regression: _IO_unbuffer_all() now invokes _IO_wsetb() to > free wide buffers of all files, including legacy standard files that > are small statically allocated objects that do not have wide buffers. Maybe at “and the _mode member”? Does the crash reproduce under mtrace? Then perhaps we can create a test case by hiding the _IO_stdin_used symbol. > diff --git a/libio/genops.c b/libio/genops.c > index 2a0d9b81df..c53696f2e0 100644 > --- a/libio/genops.c > +++ b/libio/genops.c > @@ -816,8 +816,11 @@ _IO_unbuffer_all (void) > > _IO_SETBUF (fp, NULL, 0); > > - if (fp->_mode > 0) > - _IO_wsetb (fp, NULL, NULL, 0); > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > + if (!_IO_legacy_file (fp)) > +#endif > + if (fp->_mode > 0) > + _IO_wsetb (fp, NULL, NULL, 0); I can change my patch to always define _IO_legacy_file, for newer ABI baselines as constantly return false. Thanks, Florian
On Mon, Feb 18, 2019 at 01:56:47PM +0100, Florian Weimer wrote: > * Dmitry V. Levin: > > > Commit glibc-2.23~693 (a601b74d31ca086de38441d316a3dee24c866305) > > introduced a regression: _IO_unbuffer_all() now invokes _IO_wsetb() to > > free wide buffers of all files, including legacy standard files that > > are small statically allocated objects that do not have wide buffers. > > Maybe at “and the _mode member”? Yes, the _mode member is also not available. > Does the crash reproduce under mtrace? Then perhaps we can create a > test case by hiding the _IO_stdin_used symbol. Yes and no. Apparently, this simple test crashes under mtrace: $ cat tst-bz24228.c #include <mcheck.h> int main() { mtrace(); return 0; } $ cat tst-bz24228.map { local: _IO_stdin_used; }; $ gcc -Wall -O2 -Wl,--version-script,tst-bz24228.map tst-bz24228.c $ MALLOC_TRACE=/dev/null ./a.out Segmentation fault But the crash is caused by a different issue that arises when _IO_stdin_used == NULL. > > diff --git a/libio/genops.c b/libio/genops.c > > index 2a0d9b81df..c53696f2e0 100644 > > --- a/libio/genops.c > > +++ b/libio/genops.c > > @@ -816,8 +816,11 @@ _IO_unbuffer_all (void) > > > > _IO_SETBUF (fp, NULL, 0); > > > > - if (fp->_mode > 0) > > - _IO_wsetb (fp, NULL, NULL, 0); > > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > > + if (!_IO_legacy_file (fp)) > > +#endif > > + if (fp->_mode > 0) > > + _IO_wsetb (fp, NULL, NULL, 0); Oops, this change makes misc/tst-error1-mem fail again for exactly the same reason that led to commit glibc-2.23~693. > I can change my patch to always define _IO_legacy_file, for newer ABI > baselines as constantly return false. Yes, that would be handy, but not necessary.
On Mon, Feb 18, 2019 at 10:10:21PM +0300, Dmitry V. Levin wrote: > On Mon, Feb 18, 2019 at 01:56:47PM +0100, Florian Weimer wrote: > > * Dmitry V. Levin: > > > > > Commit glibc-2.23~693 (a601b74d31ca086de38441d316a3dee24c866305) > > > introduced a regression: _IO_unbuffer_all() now invokes _IO_wsetb() to > > > free wide buffers of all files, including legacy standard files that > > > are small statically allocated objects that do not have wide buffers. > > > > Maybe at “and the _mode member”? > > Yes, the _mode member is also not available. > > > Does the crash reproduce under mtrace? Then perhaps we can create a > > test case by hiding the _IO_stdin_used symbol. > > Yes and no. Apparently, this simple test crashes under mtrace: > > $ cat tst-bz24228.c > #include <mcheck.h> > int main() { mtrace(); return 0; } > $ cat tst-bz24228.map > { local: _IO_stdin_used; }; > $ gcc -Wall -O2 -Wl,--version-script,tst-bz24228.map tst-bz24228.c > $ MALLOC_TRACE=/dev/null ./a.out > Segmentation fault This memory corruption is caused by "fp->_mode = -1;" statement in _IO_unbuffer_all(). I think we should avoid touching legacy standard files in compatibility mode. A fix with a test follows.
diff --git a/libio/genops.c b/libio/genops.c index 2a0d9b81df..c53696f2e0 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -816,8 +816,11 @@ _IO_unbuffer_all (void) _IO_SETBUF (fp, NULL, 0); - if (fp->_mode > 0) - _IO_wsetb (fp, NULL, NULL, 0); +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) + if (!_IO_legacy_file (fp)) +#endif + if (fp->_mode > 0) + _IO_wsetb (fp, NULL, NULL, 0); #ifdef _IO_MTSAFE_IO if (cnt < MAXTRIES && fp->_lock != NULL)