[08/26] stdio-common: Add tst-memstream-string for open_memstream overflow

Message ID 2d35bb583d26a0dce2d905c2cc79fb87a8e9413b.1647544751.git.fweimer@redhat.com
State Committed
Headers
Series vfprintf rework to remove vtables |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer March 17, 2022, 7:29 p.m. UTC
  This code path is exercised indirectly by some of the DNS stub
resolver tests, via their own use of xopen_memstream for constructing
strings describing result data.  The relative lack of test suite
coverage became apparent when these tests starting failing after a
printf changes uncovered bug 28949.
---
 stdio-common/Makefile               |  1 +
 stdio-common/tst-memstream-string.c | 85 +++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 stdio-common/tst-memstream-string.c
  

Comments

Adhemerval Zanella May 20, 2022, 5:44 p.m. UTC | #1
On 17/03/2022 16:29, Florian Weimer via Libc-alpha wrote:
> This code path is exercised indirectly by some of the DNS stub
> resolver tests, via their own use of xopen_memstream for constructing
> strings describing result data.  The relative lack of test suite
> coverage became apparent when these tests starting failing after a
> printf changes uncovered bug 28949.

LGTM, with maybe a suggestio below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  stdio-common/Makefile               |  1 +
>  stdio-common/tst-memstream-string.c | 85 +++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
>  create mode 100644 stdio-common/tst-memstream-string.c
> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index f0e65f0dcd..222c9ea63d 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -173,6 +173,7 @@ tests := \
>    tst-gets \
>    tst-grouping \
>    tst-long-dbl-fphex \
> +  tst-memstream-string \
>    tst-obprintf \
>    tst-perror \
>    tst-popen \
> diff --git a/stdio-common/tst-memstream-string.c b/stdio-common/tst-memstream-string.c
> new file mode 100644
> index 0000000000..1c1bf0154a
> --- /dev/null
> +++ b/stdio-common/tst-memstream-string.c
> @@ -0,0 +1,85 @@
> +/* Test writing differently sized strings to a memstream.
> +   Copyright (C) 2022 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xmemstream.h>
> +
> +/* Returns a printable ASCII character based on INDEX.   */
> +static inline char
> +char_from_index (unsigned int index)
> +{
> +  return ' ' + (index % 95);
> +}
> +
> +/* Hide fprintf behind a compiler barrier, to avoid the fputs
> +   transformation.  */
> +void __attribute__ ((weak))
> +fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg)
> +{
> +  fprintf (fp, format, arg);
> +}

You can also avoid it by explicit disable the builtin on object build:

CFLAGS-tst-memstream-string.c += -fno-builtin-fprintf

> +
> +enum { result_size = 25000 };
> +
> +static void
> +run_one_size (unsigned int chunk_size)
> +{
> +  char *chunk = xmalloc (chunk_size + 1);
> +
> +  struct xmemstream mem;
> +  xopen_memstream (&mem);
> +  unsigned int written = 0;
> +  for (unsigned int i = 0; i < result_size; )
> +    {
> +      unsigned int to_print = result_size - i;
> +      if (to_print > chunk_size)
> +        to_print = chunk_size;
> +      for (unsigned int j = 0; j < to_print; ++j)
> +        chunk[j] = char_from_index(i + j);
> +      chunk[to_print] = '\0';
> +      fprintf_compiler_barrier (mem.out, "%s", chunk);
> +      i += to_print;
> +      written += strlen(chunk);
> +    }
> +  xfclose_memstream (&mem);
> +
> +  TEST_COMPARE (written, result_size);
> +  TEST_COMPARE (mem.length, result_size);
> +  TEST_COMPARE (strlen (mem.buffer), result_size);
> +
> +  for (unsigned int i = 0; i < result_size; ++i)
> +    TEST_COMPARE (mem.buffer[i], char_from_index (i));
> +
> +  free (mem.buffer);
> +  free (chunk);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  for (unsigned int chunk_size = 1; chunk_size <= 30; ++ chunk_size)
> +    run_one_size (chunk_size);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
  
Florian Weimer May 23, 2022, 7:03 a.m. UTC | #2
* Adhemerval Zanella:

> On 17/03/2022 16:29, Florian Weimer via Libc-alpha wrote:
>> This code path is exercised indirectly by some of the DNS stub
>> resolver tests, via their own use of xopen_memstream for constructing
>> strings describing result data.  The relative lack of test suite
>> coverage became apparent when these tests starting failing after a
>> printf changes uncovered bug 28949.
>
> LGTM, with maybe a suggestio below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

>> +/* Hide fprintf behind a compiler barrier, to avoid the fputs
>> +   transformation.  */
>> +void __attribute__ ((weak))
>> +fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg)
>> +{
>> +  fprintf (fp, format, arg);
>> +}
>
> You can also avoid it by explicit disable the builtin on object build:
>
> CFLAGS-tst-memstream-string.c += -fno-builtin-fprintf

Thanks, I made this change.

Florian
  

Patch

diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index f0e65f0dcd..222c9ea63d 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -173,6 +173,7 @@  tests := \
   tst-gets \
   tst-grouping \
   tst-long-dbl-fphex \
+  tst-memstream-string \
   tst-obprintf \
   tst-perror \
   tst-popen \
diff --git a/stdio-common/tst-memstream-string.c b/stdio-common/tst-memstream-string.c
new file mode 100644
index 0000000000..1c1bf0154a
--- /dev/null
+++ b/stdio-common/tst-memstream-string.c
@@ -0,0 +1,85 @@ 
+/* Test writing differently sized strings to a memstream.
+   Copyright (C) 2022 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xmemstream.h>
+
+/* Returns a printable ASCII character based on INDEX.   */
+static inline char
+char_from_index (unsigned int index)
+{
+  return ' ' + (index % 95);
+}
+
+/* Hide fprintf behind a compiler barrier, to avoid the fputs
+   transformation.  */
+void __attribute__ ((weak))
+fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg)
+{
+  fprintf (fp, format, arg);
+}
+
+enum { result_size = 25000 };
+
+static void
+run_one_size (unsigned int chunk_size)
+{
+  char *chunk = xmalloc (chunk_size + 1);
+
+  struct xmemstream mem;
+  xopen_memstream (&mem);
+  unsigned int written = 0;
+  for (unsigned int i = 0; i < result_size; )
+    {
+      unsigned int to_print = result_size - i;
+      if (to_print > chunk_size)
+        to_print = chunk_size;
+      for (unsigned int j = 0; j < to_print; ++j)
+        chunk[j] = char_from_index(i + j);
+      chunk[to_print] = '\0';
+      fprintf_compiler_barrier (mem.out, "%s", chunk);
+      i += to_print;
+      written += strlen(chunk);
+    }
+  xfclose_memstream (&mem);
+
+  TEST_COMPARE (written, result_size);
+  TEST_COMPARE (mem.length, result_size);
+  TEST_COMPARE (strlen (mem.buffer), result_size);
+
+  for (unsigned int i = 0; i < result_size; ++i)
+    TEST_COMPARE (mem.buffer[i], char_from_index (i));
+
+  free (mem.buffer);
+  free (chunk);
+}
+
+static int
+do_test (void)
+{
+  for (unsigned int chunk_size = 1; chunk_size <= 30; ++ chunk_size)
+    run_one_size (chunk_size);
+
+  return 0;
+}
+
+#include <support/test-driver.c>