Re: [PATCH] Updated buffer overflow protection patch

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Oct 26, 2004 at 03:49:52PM -0400, Diego Novillo wrote:
> I think I've asked you before, but why do we rely on the code having
> calls to mem*_chk instead of figuring it out ourselves?  If we know the

1) you need to tell GCC somehow that your C library (or some aux library)
   provides the functions that check the size
2) you need to tell GCC whether you want stringops checked or not
3) the patch distinguishes between whole objects and subobjects (closest
   enclosing field etc.), so you need to tell GCC whether you want
   to fail whenever the whole object or subobject is overflown

With the __builtin_object_size () builtin, you can handle checking of your
own functions too and in fact can handle the normal stringops easily too.
Furthermore, whether checking is done for particular function can be
more fine grained.

E.g. glibc checks gets (if requested via -D_FORTIFY_SOURCE):
extern char *__gets_chk (char *__str, size_t);
#define gets(__str) \
  ((__bos (__str) == (size_t) -1)                               \
   ? (gets) (__str) : __gets_chk (__str, __bos (__str)))

or say memcpy could be done with:
extern void *__memcpy_chk (void *__restrict, const void *__restrict, size_t);
#define memcpy(dest, src, len) \
  ((__bos0 (dest) != (size_t) -1				\
    || (__builtin_constant_p (len) && len <= __bos0 (dest)))	\
   ? (memcpy) (dest, src, len) : __memcpy_chk (dest, src, len))

You can easily code checking variant of strlcpy, g_strlcpy and whatever
other function is often used and could benefit from checking.

The reason why __builtin_memcpy_chk, __builtin_strcpy_chk have been added
is the same as why __builtin_memcpy or __builtin_strcpy are in GCC
- GCC knows what the functions do and can do some optimizations on them
that are not easily doable from macros/inlines.
But I think it is better when the interface is the same.
Furthermore, it allows you to distinguish 3), as in:
struct A { char buf[10]; char buf2[120]; } a;
strcpy (&a.buf[2], b);
when compiled with -D_FORTIFY_SOURCE=1, this will fail if
strlen (b) + 1 >= 128, while with -D_FORTIFY_SOURCE=2 will fail already
if strlen (b) + 1 >= 8.

> Also, the propagation of these lengths is only extending the hack we had
> for strlen and friends.  We need to propagate string attributes such as
> length using the generic propagator.

I briefly looked at using the generic propagator but couldn't see how
it matches the needs of object size propagation.
First of all, I'd like to understand whether you want to propagate all
attributes in one pass or have separate passes for different attributes
(say VRP, string length, CCP, object size) and just use the generic
propagator for that.
If the former, then the need of using the generic propagator is certainly
bigger.
The advantages current tree-object-size.c propagator IMHO has is that
it doesn't compute object sizes that aren't needed and it is easier to handle
cycles with DU chain loops.
tree-object-size.c propagation works in 3 separate passes.
The first pass walks the DU chains from __builtin_object_size first
arguments and computes everything unless there were DU chain loops
that matter for __builtin_object_size calls.
The second pass deals with __builtin_object_size computing minimum
within DU chain loops.  For
char buf[64], *p;
p = &buf[2];
for (i = 0; i < x; ++i)
  p += 2;
return __builtin_object_size (p, N);
if N is 0 or 1 (i.e. when computing maximum) 62 should be returned, as
pointer arithmetic must be within the object bounds (+ one past the last
byte).  If N is 2 or 3 (computing minimum), 0 should be returned, but
getting there through pass 3 could take too long.
Pass 3 is propagation of the object sizes that still need to be
computed, until no more changes happen.

	Jakub