[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
fix -fsanitize=address findings
From: |
Bruno Haible |
Subject: |
fix -fsanitize=address findings |
Date: |
Sat, 09 Mar 2019 23:31:33 +0100 |
User-agent: |
KMail/5.1.3 (Linux/4.4.0-141-generic; KDE/5.18.0; x86_64; ; ) |
Applying CC="gcc -fsanitize=address" to a gnulib testdir, I see
(aside from side memory leak reports):
ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7ffc5711adc0 at pc
0x000000402bb9 bp 0x7ffc5711ab80 sp 0x7ffc5711ab78
READ of size 8 at 0x7ffc5711adc0 thread T0
#0 0x402bb8 in rpl_strfmon_l ../../gllib/strfmon_l.c:50
#1 0x401899 in main ../../gltests/test-strfmon_l.c:47
There is already a comment
/* Hack: Consume more arguments than those that are actually given. */
in the code. Time to remove this hack, and replace it with portable code.
2019-03-09 Bruno Haible <address@hidden>
strfmon_l: Fix -fsanitize=address finding.
* lib/strfmon_l.c: Include <errno.h>, <stdbool.h>, <stdlib.h>,
<string.h>.
(MAX_ARGS): Renamed from MAX_ARG_WORDS.
(directive_t, directives_t): New types.
(fmon_parse): New function.
(rpl_strfmon_l): Don't call va_arg more often than needed for the
format string. Consume 'long double' arguments in places where the
format string indicates so.
* modules/strfmon_l (Depends-on): Add 'stdbool'.
diff --git a/lib/strfmon_l.c b/lib/strfmon_l.c
index b388c88..7f1d28f 100644
--- a/lib/strfmon_l.c
+++ b/lib/strfmon_l.c
@@ -19,13 +19,103 @@
/* Specification. */
#include <monetary.h>
+#include <errno.h>
#include <locale.h>
#include <stdarg.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
#undef strfmon_l
/* This override can only support a limited number of arguments. */
-#define MAX_ARG_WORDS 16
+#define MAX_ARGS 16
+
+/* A parsed directive. */
+typedef struct
+{
+ bool needs_long_double;
+ const char *conversion_ptr;
+}
+directive_t;
+
+/* A parsed format string. */
+typedef struct
+{
+ size_t count;
+ directive_t dir[MAX_ARGS];
+}
+directives_t;
+
+/* Parses a monetary format string.
+ Returns 0 and fills *DIRECTIVESP if valid.
+ Returns -1 if invalid. */
+static int
+fmon_parse (const char *format, directives_t *directivesp)
+{
+ size_t count = 0;
+ const char *cp = format;
+
+ while (*cp != '\0')
+ {
+ if (*cp++ == '%')
+ {
+ /* Parse flags. */
+ while (*cp == '=' || *cp == '^' || *cp == '+' || *cp == '('
+ || *cp == '!' || *cp == '-')
+ {
+ if (*cp == '=')
+ {
+ cp++;
+ if (*cp == '\0')
+ return -1;
+ }
+ cp++;
+ }
+ /* Parse field width. */
+ while (*cp >= '0' && *cp <= '9')
+ cp++;
+ /* Parse left precision. */
+ if (*cp == '#')
+ {
+ cp++;
+ while (*cp >= '0' && *cp <= '9')
+ cp++;
+ }
+ /* Parse right precision. */
+ if (*cp == '.')
+ {
+ cp++;
+ while (*cp >= '0' && *cp <= '9')
+ cp++;
+ }
+ /* Now comes the conversion specifier. */
+ if (*cp != '%')
+ {
+ if (count == MAX_ARGS)
+ /* Too many arguments. */
+ return -1;
+
+ /* glibc supports an 'L' modifier before the conversion
specifier. */
+ if (*cp == 'L')
+ {
+ cp++;
+ directivesp->dir[count].needs_long_double = true;
+ }
+ else
+ directivesp->dir[count].needs_long_double = false;
+ if (!(*cp == 'i' || *cp == 'n'))
+ return -1;
+ directivesp->dir[count].conversion_ptr = cp;
+ count++;
+ }
+ cp++;
+ }
+ }
+
+ directivesp->count = count;
+ return 0;
+}
ssize_t
rpl_strfmon_l (char *s, size_t maxsize, locale_t locale, const char *format,
...)
@@ -33,9 +123,8 @@ rpl_strfmon_l (char *s, size_t maxsize, locale_t locale,
const char *format, ...
/* Work around glibc 2.23 bug
<https://sourceware.org/bugzilla/show_bug.cgi?id=19633>. */
va_list argptr;
- double args[MAX_ARG_WORDS];
- int i;
locale_t orig_locale;
+ directives_t directives;
ssize_t result;
orig_locale = uselocale ((locale_t)0);
@@ -44,17 +133,101 @@ rpl_strfmon_l (char *s, size_t maxsize, locale_t locale,
const char *format, ...
/* errno is set. */
return -1;
- va_start (argptr, format);
- /* Hack: Consume more arguments than those that are actually given. */
- for (i = 0; i < MAX_ARG_WORDS; i++)
- args[i] = va_arg (argptr, double);
-
- result = strfmon_l (s, maxsize, locale, format,
- args[0], args[1], args[2], args[3], args[4], args[5],
- args[6], args[7], args[8], args[9], args[10], args[11],
- args[12], args[13], args[14], args[15]);
-
- va_end (argptr);
+ /* The format string may consume 'double' or 'long double' arguments.
+ In order not to have to link with libffcall or libffi, convert all
+ arguments to 'long double', and use a modified format string that
+ requests 'long double' arguments. But since 'long double' arguments
+ are only supported on glibc, do so only if the original format string
+ consumes at least one 'long double' argument. */
+ if (fmon_parse (format, &directives) < 0)
+ {
+ errno = EINVAL;
+ result = -1;
+ }
+ else
+ {
+ bool use_long_double;
+ unsigned int i;
+
+ use_long_double = false;
+ for (i = 0; i < directives.count; i++)
+ if (directives.dir[i].needs_long_double)
+ {
+ use_long_double = true;
+ break;
+ }
+
+ va_start (argptr, format);
+
+ if (use_long_double)
+ {
+ char *ld_format;
+
+ /* Allocate room for the modified format string. */
+ ld_format = (char *) malloc (strlen (format) + directives.count + 1);
+ if (ld_format == NULL)
+ {
+ errno = ENOMEM;
+ result = -1;
+ }
+ else
+ {
+ long double args[MAX_ARGS];
+
+ /* Create the modified format string. */
+ {
+ const char *p = format;
+ char *dest = ld_format;
+ for (i = 0; i < directives.count; i++)
+ {
+ const char *q = directives.dir[i].conversion_ptr;
+ memcpy (dest, p, q - p);
+ dest += q - p;
+ if (!directives.dir[i].needs_long_double)
+ *dest++ = 'L';
+ p = q;
+ }
+ strcpy (dest, p);
+ }
+
+ /* Set up arguments array. */
+ for (i = 0; i < directives.count; i++)
+ args[i] = (directives.dir[i].needs_long_double
+ ? va_arg (argptr, long double)
+ : (long double) va_arg (argptr, double));
+ /* Avoid uninitialized memory references. */
+ for (; i < MAX_ARGS; i++)
+ args[i] = 0.0L;
+
+ result = strfmon_l (s, maxsize, locale, ld_format,
+ args[0], args[1], args[2], args[3], args[4],
+ args[5], args[6], args[7], args[8], args[9],
+ args[10], args[11], args[12], args[13],
+ args[14], args[15]);
+
+ free (ld_format);
+ }
+ }
+ else
+ {
+ double args[MAX_ARGS];
+
+ /* Set up arguments array. */
+ for (i = 0; i < directives.count; i++)
+ args[i] = va_arg (argptr, double);
+ /* Avoid uninitialized memory references. */
+ for (; i < MAX_ARGS; i++)
+ args[i] = 0.0;
+
+ result = strfmon_l (s, maxsize, locale, format,
+ args[0], args[1], args[2], args[3], args[4],
+ args[5], args[6], args[7], args[8], args[9],
+ args[10], args[11], args[12], args[13], args[14],
+ args[15]);
+ }
+
+ va_end (argptr);
+ }
if (uselocale (orig_locale) == (locale_t)0)
/* errno is set. */
diff --git a/modules/strfmon_l b/modules/strfmon_l
index 42a4c96..16c339c 100644
--- a/modules/strfmon_l
+++ b/modules/strfmon_l
@@ -8,6 +8,7 @@ m4/strfmon_l.m4
Depends-on:
monetary
extensions
+stdbool [test $REPLACE_STRFMON_L = 1]
configure.ac:
gl_FUNC_STRFMON_L
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- fix -fsanitize=address findings,
Bruno Haible <=