[PATCH v6 11/11] libio: Convert __vswprintf_internal to buffers (bug 27857)
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Dec 19 17:53:13 GMT 2022
On 16/12/22 17:15, Florian Weimer via Libc-alpha wrote:
> Always null-terminate the buffer and set E2BIG if the buffer is too
> small. This fixes bug 27857.
Good, another vtable removed.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/Makefile | 1 -
> include/printf_buffer.h | 1 +
> libio/tst_swprintf.c | 31 ++++++++-
> libio/vswprintf.c | 100 +++++-----------------------
> manual/stdio.texi | 7 +-
> stdio-common/wprintf_buffer_flush.c | 6 ++
> 6 files changed, 57 insertions(+), 89 deletions(-)
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 36901ac3ab..0ecbcde962 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -609,7 +609,6 @@ $(objpfx)tst-relro-libc.out: tst-relro-symbols.py $(..)/scripts/glibcelf.py \
> --required=_IO_wmem_jumps \
> --required=_IO_wprintf_buffer_as_file_jumps \
> --required=_IO_wstr_jumps \
> - --required=_IO_wstrn_jumps \
> --optional=_IO_old_cookie_jumps \
> --optional=_IO_old_file_jumps \
> --optional=_IO_old_proc_jumps \
> diff --git a/include/printf_buffer.h b/include/printf_buffer.h
> index 168555d8d3..e7a45fb45f 100644
> --- a/include/printf_buffer.h
> +++ b/include/printf_buffer.h
> @@ -196,6 +196,7 @@ bool __printf_buffer_flush (struct __printf_buffer *buf) attribute_hidden;
> enum __wprintf_buffer_mode
> {
> __wprintf_buffer_mode_failed,
> + __wprintf_buffer_mode_swprintf,
> __wprintf_buffer_mode_to_file,
> };
>
> diff --git a/libio/tst_swprintf.c b/libio/tst_swprintf.c
> index 1f997b9393..169951de4b 100644
> --- a/libio/tst_swprintf.c
> +++ b/libio/tst_swprintf.c
> @@ -1,4 +1,5 @@
> #include <array_length.h>
> +#include <errno.h>
> #include <stdio.h>
> #include <support/check.h>
> #include <sys/types.h>
> @@ -37,6 +38,8 @@ do_test (void)
>
> for (n = 0; n < array_length (tests); ++n)
> {
> + wmemset (buf, 0xabcd, array_length (buf));
> + errno = 0;
> ssize_t res = swprintf (buf, tests[n].n, L"%s", tests[n].str);
>
> if (tests[n].exp < 0 && res >= 0)
> @@ -52,9 +55,31 @@ do_test (void)
> swprintf (buf, %zu, L\"%%s\", \"%s\") expected to return %zd, but got %zd\n",
> tests[n].n, tests[n].str, tests[n].exp, res);
> }
> - else
> - printf ("swprintf (buf, %zu, L\"%%s\", \"%s\") OK\n",
> - tests[n].n, tests[n].str);
> + else if (res < 0
> + && tests[n].n > 0
> + && wcsnlen (buf, array_length (buf)) == array_length (buf))
> + {
> + support_record_failure ();
> + printf ("\
> +error: swprintf (buf, %zu, L\"%%s\", \"%s\") missing null terminator\n",
> + tests[n].n, tests[n].str);
> + }
> + else if (res < 0
> + && tests[n].n > 0
> + && wcsnlen (buf, array_length (buf)) < array_length (buf)
> + && buf[wcsnlen (buf, array_length (buf)) + 1] != 0xabcd)
> + {
> + support_record_failure ();
> + printf ("\
> +error: swprintf (buf, %zu, L\"%%s\", \"%s\") out of bounds write\n",
> + tests[n].n, tests[n].str);
> + }
> +
> + if (res < 0 && tests[n].n < 0)
> + TEST_COMPARE (errno, E2BIG);
> +
> + printf ("swprintf (buf, %zu, L\"%%s\", \"%s\") OK\n",
> + tests[n].n, tests[n].str);
> }
>
> TEST_COMPARE (swprintf (buf, array_length (buf), L"%.0s", "foo"), 0);
> diff --git a/libio/vswprintf.c b/libio/vswprintf.c
> index 5a9ccdff3e..f53545c806 100644
> --- a/libio/vswprintf.c
> +++ b/libio/vswprintf.c
> @@ -24,101 +24,37 @@
> This exception applies to code released by its copyright holders
> in files containing the exception. */
>
> -#include "libioP.h"
> -#include "strfile.h"
> -
> -
> -static wint_t _IO_wstrn_overflow (FILE *fp, wint_t c) __THROW;
> -
> -static wint_t
> -_IO_wstrn_overflow (FILE *fp, wint_t c)
> -{
> - /* When we come to here this means the user supplied buffer is
> - filled. But since we must return the number of characters which
> - would have been written in total we must provide a buffer for
> - further use. We can do this by writing on and on in the overflow
> - buffer in the _IO_wstrnfile structure. */
> - _IO_wstrnfile *snf = (_IO_wstrnfile *) fp;
> -
> - if (fp->_wide_data->_IO_buf_base != snf->overflow_buf)
> - {
> - _IO_wsetb (fp, snf->overflow_buf,
> - snf->overflow_buf + (sizeof (snf->overflow_buf)
> - / sizeof (wchar_t)), 0);
> -
> - fp->_wide_data->_IO_write_base = snf->overflow_buf;
> - fp->_wide_data->_IO_read_base = snf->overflow_buf;
> - fp->_wide_data->_IO_read_ptr = snf->overflow_buf;
> - fp->_wide_data->_IO_read_end = (snf->overflow_buf
> - + (sizeof (snf->overflow_buf)
> - / sizeof (wchar_t)));
> - }
> -
> - fp->_wide_data->_IO_write_ptr = snf->overflow_buf;
> - fp->_wide_data->_IO_write_end = snf->overflow_buf;
> -
> - /* Since we are not really interested in storing the characters
> - which do not fit in the buffer we simply ignore it. */
> - return c;
> -}
> -
> -
> -const struct _IO_jump_t _IO_wstrn_jumps libio_vtable attribute_hidden =
> -{
> - JUMP_INIT_DUMMY,
> - JUMP_INIT(finish, _IO_wstr_finish),
> - JUMP_INIT(overflow, (_IO_overflow_t) _IO_wstrn_overflow),
> - JUMP_INIT(underflow, (_IO_underflow_t) _IO_wstr_underflow),
> - JUMP_INIT(uflow, (_IO_underflow_t) _IO_wdefault_uflow),
> - JUMP_INIT(pbackfail, (_IO_pbackfail_t) _IO_wstr_pbackfail),
> - JUMP_INIT(xsputn, _IO_wdefault_xsputn),
> - JUMP_INIT(xsgetn, _IO_wdefault_xsgetn),
> - JUMP_INIT(seekoff, _IO_wstr_seekoff),
> - JUMP_INIT(seekpos, _IO_default_seekpos),
> - JUMP_INIT(setbuf, _IO_default_setbuf),
> - JUMP_INIT(sync, _IO_default_sync),
> - JUMP_INIT(doallocate, _IO_wdefault_doallocate),
> - JUMP_INIT(read, _IO_default_read),
> - JUMP_INIT(write, _IO_default_write),
> - JUMP_INIT(seek, _IO_default_seek),
> - JUMP_INIT(close, _IO_default_close),
> - JUMP_INIT(stat, _IO_default_stat),
> - JUMP_INIT(showmanyc, _IO_default_showmanyc),
> - JUMP_INIT(imbue, _IO_default_imbue)
> -};
> -
> +#include <errno.h>
> +#include <math_ldbl_opt.h>
> +#include <printf.h>
> +#include <printf_buffer.h>
>
> int
> __vswprintf_internal (wchar_t *string, size_t maxlen, const wchar_t *format,
> va_list args, unsigned int mode_flags)
> {
> - _IO_wstrnfile sf;
> - int ret;
> - struct _IO_wide_data wd;
> -#ifdef _IO_MTSAFE_IO
> - sf.f._sbf._f._lock = NULL;
> -#endif
> -
> if (maxlen == 0)
> /* Since we have to write at least the terminating L'\0' a buffer
> length of zero always makes the function fail. */
> return -1;
>
> - _IO_no_init (&sf.f._sbf._f, _IO_USER_LOCK, 0, &wd, &_IO_wstrn_jumps);
> - _IO_fwide (&sf.f._sbf._f, 1);
> - string[0] = L'\0';
> - _IO_wstr_init_static (&sf.f._sbf._f, string, maxlen - 1, string);
> - ret = __vfwprintf_internal ((FILE *) &sf.f._sbf, format, args, mode_flags);
> + struct __wprintf_buffer buf;
> + __wprintf_buffer_init (&buf, string, maxlen, __wprintf_buffer_mode_swprintf);
>
> - if (sf.f._sbf._f._wide_data->_IO_buf_base == sf.overflow_buf)
> - /* ISO C99 requires swprintf/vswprintf to return an error if the
> - output does not fit in the provided buffer. */
> - return -1;
> + __wprintf_buffer (&buf, format, args, mode_flags);
> +
> + if (buf.write_ptr == buf.write_end)
> + {
> + /* Buffer has been filled exactly, excluding the null wide
> + character. This is an error because the null wide character
> + is required. */
> + buf.write_end[-1] = L'\0';
> + return -1;
> + }
>
> - /* Terminate the string. */
> - *sf.f._sbf._f._wide_data->_IO_write_ptr = '\0';
> + buf.write_ptr[0] = L'\0';
>
> - return ret;
> + return __wprintf_buffer_done (&buf);
> }
>
> int
> diff --git a/manual/stdio.texi b/manual/stdio.texi
> index 4aa1a2bc2d..f6319a4b8a 100644
> --- a/manual/stdio.texi
> +++ b/manual/stdio.texi
> @@ -2412,9 +2412,10 @@ allocate at least @var{size} wide characters for the string @var{ws}.
>
> The return value is the number of characters generated for the given
> input, excluding the trailing null. If not all output fits into the
> -provided buffer a negative value is returned. You should try again with
> -a bigger output string. @emph{Note:} this is different from how
> -@code{snprintf} handles this situation.
> +provided buffer a negative value is returned, and @code{errno} is set to
> +@code{E2BIG}. (The setting of @code{errno} is a GNU extension.) You
> +should try again with a bigger output string. @emph{Note:} this is
> +different from how @code{snprintf} handles this situation.
>
> Note that the corresponding narrow stream function takes fewer
> parameters. @code{swprintf} in fact corresponds to the @code{snprintf}
> diff --git a/stdio-common/wprintf_buffer_flush.c b/stdio-common/wprintf_buffer_flush.c
> index 2d91095cca..46af0f348f 100644
> --- a/stdio-common/wprintf_buffer_flush.c
> +++ b/stdio-common/wprintf_buffer_flush.c
> @@ -16,6 +16,7 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <errno.h>
> #include <printf_buffer.h>
>
> #include "printf_buffer-wchar_t.h"
> @@ -31,6 +32,11 @@ __wprintf_buffer_do_flush (struct __wprintf_buffer *buf)
> case __wprintf_buffer_mode_to_file:
> __wprintf_buffer_flush_to_file ((struct __wprintf_buffer_to_file *) buf);
> return;
> + case __wprintf_buffer_mode_swprintf:
> + buf->write_end[-1] = L'\0';
> + __set_errno (E2BIG);
> + __wprintf_buffer_mark_failed (buf);
> + return;
> }
> __builtin_trap ();
> }
More information about the Libc-alpha
mailing list