Fix crash in _IO_wfile_sync (bug 20568)
Commit Message
[BZ #20568]
* libio/wfileops.c (_IO_wfile_sync): Correct last argument to
__codecvt_do_length.
* libio/Makefile (tests): Add tst-wfile-sync.
($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales).
* libio/tst-wfile-sync.c: New file.
* libio/tst-wfile-sync.input: New file.
---
libio/Makefile | 4 +++-
libio/tst-wfile-sync.c | 35 +++++++++++++++++++++++++++++++++++
libio/tst-wfile-sync.input | 1 +
libio/wfileops.c | 4 ++--
4 files changed, 41 insertions(+), 3 deletions(-)
create mode 100644 libio/tst-wfile-sync.c
create mode 100644 libio/tst-wfile-sync.input
Comments
* Andreas Schwab:
> [BZ #20568]
> * libio/wfileops.c (_IO_wfile_sync): Correct last argument to
> __codecvt_do_length.
> * libio/Makefile (tests): Add tst-wfile-sync.
> ($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales).
> * libio/tst-wfile-sync.c: New file.
> * libio/tst-wfile-sync.input: New file.
Would you please reference commit
18d26750dd8fd328a78cf639fd0ec2494680a2a4 in the commit message?
> diff --git a/libio/Makefile b/libio/Makefile
> index 95b91084dd..a5236c7042 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -65,7 +65,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
> tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
> tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
> tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
> - tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153
> + tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
> + tst-wfile-sync
Okay.
> @@ -212,6 +213,7 @@ $(objpfx)tst-ungetwc1.out: $(gen-locales)
> $(objpfx)tst-ungetwc2.out: $(gen-locales)
> $(objpfx)tst-widetext.out: $(gen-locales)
> $(objpfx)tst_wprintf2.out: $(gen-locales)
> +$(objpfx)tst-wfile-sync.out: $(gen-locales)
> endif
Okay.
> $(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen
> diff --git a/libio/tst-wfile-sync.c b/libio/tst-wfile-sync.c
> new file mode 100644
> index 0000000000..e1f621ede7
> --- /dev/null
> +++ b/libio/tst-wfile-sync.c
> @@ -0,0 +1,35 @@
> +/* Test that _IO_wfile_sync does not crash (bug 20568).
> + Copyright (C) 2016-2019 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <locale.h>
> +#include <stdio.h>
> +#include <wchar.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> + TEST_VERIFY_EXIT (setlocale (LC_ALL, "de_DE.UTF-8") != NULL);
> + TEST_VERIFY_EXIT (fgetwc (stdin) != WEOF);
> + /* This calls _IO_wfile_sync. */
> + TEST_VERIFY_EXIT (setvbuf (stdin, NULL, _IONBF, 0) == 0);
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
You could perhaps add
TEST_COMPARE (xlseek (0, 0, SEEK_CUR), 1);
, to show that the underlying descriptor was updated correctly.
> diff --git a/libio/tst-wfile-sync.input b/libio/tst-wfile-sync.input
> new file mode 100644
> index 0000000000..12d0958f7a
> --- /dev/null
> +++ b/libio/tst-wfile-sync.input
> @@ -0,0 +1 @@
> +This is a test of _IO_wfile_sync.
Woah. Magic! I didn't know we had this Makefile rule:
$(objpfx)%.out: %.input $(objpfx)%
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index 0367643703..42aaf5e855 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -508,11 +508,11 @@ _IO_wfile_sync (FILE *fp)
> generate the wide characters up to the current reading
> position. */
> int nread;
> -
> + size_t wnread = fp->_wide_data->_IO_read_ptr - fp->_wide_data->_IO_read_base;
> fp->_wide_data->_IO_state = fp->_wide_data->_IO_last_state;
> nread = (*cv->__codecvt_do_length) (cv, &fp->_wide_data->_IO_state,
> fp->_IO_read_base,
> - fp->_IO_read_end, delta);
> + fp->_IO_read_end, wnread);
> fp->_IO_read_ptr = fp->_IO_read_base + nread;
> delta = -(fp->_IO_read_end - fp->_IO_read_base - nread);
> }
Okay.
The fixed-width case is not affected because there, the seek amount can
be determined directly from the distance between _IO_read_ptr and
_IO_read_end.
Thanks,
Florian
On Mai 15 2019, Florian Weimer <fweimer@redhat.com> wrote:
> * Andreas Schwab:
>
>> [BZ #20568]
>> * libio/wfileops.c (_IO_wfile_sync): Correct last argument to
>> __codecvt_do_length.
>> * libio/Makefile (tests): Add tst-wfile-sync.
>> ($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales).
>> * libio/tst-wfile-sync.c: New file.
>> * libio/tst-wfile-sync.input: New file.
>
> Would you please reference commit
> 18d26750dd8fd328a78cf639fd0ec2494680a2a4 in the commit message?
It's related, but not necessary. The test will crash without that.
Andreas.
* Andreas Schwab:
> On Mai 15 2019, Florian Weimer <fweimer@redhat.com> wrote:
>
>> * Andreas Schwab:
>>
>>> [BZ #20568]
>>> * libio/wfileops.c (_IO_wfile_sync): Correct last argument to
>>> __codecvt_do_length.
>>> * libio/Makefile (tests): Add tst-wfile-sync.
>>> ($(objpfx)tst-wfile-sync.out): Depend on $(gen-locales).
>>> * libio/tst-wfile-sync.c: New file.
>>> * libio/tst-wfile-sync.input: New file.
>>
>> Would you please reference commit
>> 18d26750dd8fd328a78cf639fd0ec2494680a2a4 in the commit message?
>
> It's related, but not necessary. The test will crash without that.
Ahh. It might still make sense to mention that the bug becomes more
visible after that commit. But the bug reference probably covers that.
Thanks,
Florian
@@ -65,7 +65,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \
tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
- tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153
+ tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
+ tst-wfile-sync
tests-internal = tst-vtables tst-vtables-interposed tst-readline
@@ -212,6 +213,7 @@ $(objpfx)tst-ungetwc1.out: $(gen-locales)
$(objpfx)tst-ungetwc2.out: $(gen-locales)
$(objpfx)tst-widetext.out: $(gen-locales)
$(objpfx)tst_wprintf2.out: $(gen-locales)
+$(objpfx)tst-wfile-sync.out: $(gen-locales)
endif
$(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen
new file mode 100644
@@ -0,0 +1,35 @@
+/* Test that _IO_wfile_sync does not crash (bug 20568).
+ Copyright (C) 2016-2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <locale.h>
+#include <stdio.h>
+#include <wchar.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+ TEST_VERIFY_EXIT (setlocale (LC_ALL, "de_DE.UTF-8") != NULL);
+ TEST_VERIFY_EXIT (fgetwc (stdin) != WEOF);
+ /* This calls _IO_wfile_sync. */
+ TEST_VERIFY_EXIT (setvbuf (stdin, NULL, _IONBF, 0) == 0);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1 @@
+This is a test of _IO_wfile_sync.
@@ -508,11 +508,11 @@ _IO_wfile_sync (FILE *fp)
generate the wide characters up to the current reading
position. */
int nread;
-
+ size_t wnread = fp->_wide_data->_IO_read_ptr - fp->_wide_data->_IO_read_base;
fp->_wide_data->_IO_state = fp->_wide_data->_IO_last_state;
nread = (*cv->__codecvt_do_length) (cv, &fp->_wide_data->_IO_state,
fp->_IO_read_base,
- fp->_IO_read_end, delta);
+ fp->_IO_read_end, wnread);
fp->_IO_read_ptr = fp->_IO_read_base + nread;
delta = -(fp->_IO_read_end - fp->_IO_read_base - nread);
}