[PATCH RFC] __builtin_dynamic_object_size with -D_FORTIFY_SOURCE=3
Siddhesh Poyarekar
siddhesh@sourceware.org
Mon Nov 30 08:40:53 GMT 2020
I tried to incorporate the __builtin_dynamic_object_size[1] builtin
available in clang to enhance _FORTIFY_SOURCE and was looking for some
early feedback so that I can incorporate it into the final patch that
I hope to come up with soon.
__builtin_dynamic_object_size
-----------------------------
__builtin_dynamic_object_size is intended to be a drop-in replacement
for __builtin_object_size (more on why it isn't at the moment, but
later) that goes a little further. In addition to what
__builtin_object_size does, i.e. replace the builtin call with a
constant object size, __builtin_dynamic_object_size will replace the
call site with an expression that evaluates to the object size, thus
expanding its applicability by a bit. In practice, it can find these
expressions through malloc/calloc calls earlier in the translation
unit.
A simple motivating example is below; -D_FORTIFY_SOURCE=2 would miss
this and emit memcpy, but -D_FORTIFY_SOURCE=3 with the help of
__builtin_dynamic_object_size is able to emit __memcpy_chk with the
allocation size expression passed into the function:
extern void *getmem (size_t);
void *copy_obj (const void *src, size_t alloc, size_t copysize)
{
void *obj = malloc (alloc);
memcpy (obj, src, copysize);
return obj;
}
Note however that if the object was allocated elsewhere that the
compiler cannot see, or if it was allocated in the function with a
function that the compiler does not recognize as an allocator then
__builtin_dynamic_object_size also returns -1.
-D_FORTIFY_SOURCE=3
-------------------
I restricted usage of __builtin_dynamic_object_size to a new
_FORTIFY_SOURCE level because it has one fundamental difference from
the earlier level, which is that it can pass a variable to the _chk
function instead of constant. As a result, 1 and 2 are fast with most
overhead being optimized out while 3 may have additional overhead.
For example at the moment the simple case of memcpy, memmove,
etc. where they're implemented with compiler builtins like
__builtin___memcpy_chk, clang is able to generate compact code that
passes the expression to __memcpy_chk or just generates a memcpy when
possible. In the non-builtin cases though, where the size evaluates
to an expression, one may see patterns like:
cmpq $-1, %rbx
je .LBB0_2
callq fortified_chk
jmp .LBB0_3
.LBB0_2: # %if.else
callq fortified
since the compiler isn't smart enough yet to reduce that condition.
This can be fixed (I'm working on that right now) in the simple case
of a direct comparison and thus work for _FORTIFY_SOURCE=3, but it may
be harder to evaluate for __builtin_dynamic_object_size in general
where the comparison happens indirectly. This is also why
__builtin_dynamic_object_size isn't exactly a drop-in replacement for
__builtin_object_size in all cases.
Outstanding Issues/RFC:
------------------
- As mentioned above, I'm trying to get clang to eliminate that
conditional so that it can be applied more widely beyond to
__builtin___func_chk like functions. That hopefully won't be too
hard. However there is a risk that the llvm project rejects such a
peephole optimisation; in that case would the additional overhead of
the conditional be acceptable? It is a separate fortify level, so
it may be easier to justify.
- gcc does not support __builtin_dynamic_object_size yet. Based on
previous discussions[2] ISTM that the idea hasn't bee rejected
outright, just that the gcc community would like to see a more
detailed specification before committing to add it. I think the
glibc implementation will help us arrive at that, although there are
issues beyond glibc usage of __builtin_dynamic_object_size or
__builtin_object_size. I hope to start discussions on this next
year, nearer to the next gcc stage 1 open.
- The idea of -D_FORTIFY_SOURCE=3 is a beginning of a shift in design.
This introduces hardening that could potentially have noticeable
additional overhead at runtime. It does expand coverage of
fortification and there's potential for additional coverage on
similar lines. Is it enough to simply increment the fortify level
or should this be a different feature macro altogether? Maybe
incrementing the fortify level is sufficient but we need some
additional space to grow static checks, i.e. start dynamic checks
from -D_FORTIFY_SOURCE=9? Or any other concerns regarding where
such checks ought to fit in?
[1] https://reviews.llvm.org/D56760
[2] https://gcc.gnu.org/legacy-ml/gcc/2019-01/msg00188.html
---
include/features.h | 6 +++---
misc/sys/cdefs.h | 9 +++++++++
string/bits/string_fortified.h | 24 ++++++++++++------------
string/bits/strings_fortified.h | 4 ++--
4 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/include/features.h b/include/features.h
index f3e62d3362..86409dd457 100644
--- a/include/features.h
+++ b/include/features.h
@@ -397,10 +397,10 @@
# warning _FORTIFY_SOURCE requires compiling with optimization (-O)
# elif !__GNUC_PREREQ (4, 1)
# warning _FORTIFY_SOURCE requires GCC 4.1 or later
-# elif _FORTIFY_SOURCE > 1
-# define __USE_FORTIFY_LEVEL 2
+# elif _FORTIFY_SOURCE > 2
+# define __USE_FORTIFY_LEVEL 3
# else
-# define __USE_FORTIFY_LEVEL 1
+# define __USE_FORTIFY_LEVEL _FORTIFY_SOURCE
# endif
#endif
#ifndef __USE_FORTIFY_LEVEL
diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index e94d09d7dd..563b238ed5 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -127,6 +127,15 @@
#define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
#define __bos0(ptr) __builtin_object_size (ptr, 0)
+/* Use __builtin_dynamic_object_size if available. */
+#if __USE_FORTIFY_LEVEL == 3 && __glibc_clang_prereq (9, 0)
+# define __objsize0(__o) __builtin_dynamic_object_size (__o, 0)
+# define __objsize(__o) __builtin_dynamic_object_size (__o, 1)
+#else
+# define __objsize0(__o) __bos0 (__o)
+# define __objsize(__o) __bos (__o)
+#endif
+
#if __GNUC_PREREQ (4,3)
# define __warnattr(msg) __attribute__((__warning__ (msg)))
# define __errordecl(name, msg) \
diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
index 4c1aeb45f1..e8e14506f9 100644
--- a/string/bits/string_fortified.h
+++ b/string/bits/string_fortified.h
@@ -26,13 +26,13 @@ __fortify_function void *
__NTH (memcpy (void *__restrict __dest, const void *__restrict __src,
size_t __len))
{
- return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
+ return __builtin___memcpy_chk (__dest, __src, __len, __objsize0 (__dest));
}
__fortify_function void *
__NTH (memmove (void *__dest, const void *__src, size_t __len))
{
- return __builtin___memmove_chk (__dest, __src, __len, __bos0 (__dest));
+ return __builtin___memmove_chk (__dest, __src, __len, __objsize0 (__dest));
}
#ifdef __USE_GNU
@@ -40,7 +40,7 @@ __fortify_function void *
__NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
size_t __len))
{
- return __builtin___mempcpy_chk (__dest, __src, __len, __bos0 (__dest));
+ return __builtin___mempcpy_chk (__dest, __src, __len, __objsize0 (__dest));
}
#endif
@@ -53,7 +53,7 @@ __NTH (mempcpy (void *__restrict __dest, const void *__restrict __src,
__fortify_function void *
__NTH (memset (void *__dest, int __ch, size_t __len))
{
- return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
+ return __builtin___memset_chk (__dest, __ch, __len, __objsize0 (__dest));
}
#ifdef __USE_MISC
@@ -65,21 +65,21 @@ void __explicit_bzero_chk (void *__dest, size_t __len, size_t __destlen)
__fortify_function void
__NTH (explicit_bzero (void *__dest, size_t __len))
{
- __explicit_bzero_chk (__dest, __len, __bos0 (__dest));
+ __explicit_bzero_chk (__dest, __len, __objsize0 (__dest));
}
#endif
__fortify_function char *
__NTH (strcpy (char *__restrict __dest, const char *__restrict __src))
{
- return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
+ return __builtin___strcpy_chk (__dest, __src, __objsize (__dest));
}
#ifdef __USE_GNU
__fortify_function char *
__NTH (stpcpy (char *__restrict __dest, const char *__restrict __src))
{
- return __builtin___stpcpy_chk (__dest, __src, __bos (__dest));
+ return __builtin___stpcpy_chk (__dest, __src, __objsize (__dest));
}
#endif
@@ -88,14 +88,14 @@ __fortify_function char *
__NTH (strncpy (char *__restrict __dest, const char *__restrict __src,
size_t __len))
{
- return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
+ return __builtin___strncpy_chk (__dest, __src, __len, __objsize (__dest));
}
#if __GNUC_PREREQ (4, 7) || __glibc_clang_prereq (2, 6)
__fortify_function char *
__NTH (stpncpy (char *__dest, const char *__src, size_t __n))
{
- return __builtin___stpncpy_chk (__dest, __src, __n, __bos (__dest));
+ return __builtin___stpncpy_chk (__dest, __src, __n, __objsize (__dest));
}
#else
extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
@@ -107,7 +107,7 @@ extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
__fortify_function char *
__NTH (stpncpy (char *__dest, const char *__src, size_t __n))
{
- if (__bos (__dest) != (size_t) -1
+ if (__objsize (__dest) != (size_t) -1
&& (!__builtin_constant_p (__n) || __n > __bos (__dest)))
return __stpncpy_chk (__dest, __src, __n, __bos (__dest));
return __stpncpy_alias (__dest, __src, __n);
@@ -118,7 +118,7 @@ __NTH (stpncpy (char *__dest, const char *__src, size_t __n))
__fortify_function char *
__NTH (strcat (char *__restrict __dest, const char *__restrict __src))
{
- return __builtin___strcat_chk (__dest, __src, __bos (__dest));
+ return __builtin___strcat_chk (__dest, __src, __objsize (__dest));
}
@@ -126,7 +126,7 @@ __fortify_function char *
__NTH (strncat (char *__restrict __dest, const char *__restrict __src,
size_t __len))
{
- return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
+ return __builtin___strncat_chk (__dest, __src, __len, __objsize (__dest));
}
#endif /* bits/string_fortified.h */
diff --git a/string/bits/strings_fortified.h b/string/bits/strings_fortified.h
index d4091f4f69..122199e036 100644
--- a/string/bits/strings_fortified.h
+++ b/string/bits/strings_fortified.h
@@ -22,13 +22,13 @@
__fortify_function void
__NTH (bcopy (const void *__src, void *__dest, size_t __len))
{
- (void) __builtin___memmove_chk (__dest, __src, __len, __bos0 (__dest));
+ (void) __builtin___memmove_chk (__dest, __src, __len, __objsize0 (__dest));
}
__fortify_function void
__NTH (bzero (void *__dest, size_t __len))
{
- (void) __builtin___memset_chk (__dest, '\0', __len, __bos0 (__dest));
+ (void) __builtin___memset_chk (__dest, '\0', __len, __objsize0 (__dest));
}
#endif
--
2.26.2
More information about the Libc-alpha
mailing list