Message ID | 20190218124438.GB20127@altlinux.org |
---|---|
State | New |
Headers | show |
Series | libio: do not cleanup wide buffers of legacy standard files [BZ #24228] | expand |
* 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)