bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] inttostr.h: add compile-time buffer overrun checks


From: Bruno Haible
Subject: Re: [PATCH] inttostr.h: add compile-time buffer overrun checks
Date: Sun, 17 Oct 2010 15:01:07 +0200
User-agent: KMail/1.9.9

Hi Jim,

> @smallexample
> #undef memcpy
> #define bos0(dest) __builtin_object_size (dest, 0)
> #define memcpy(dest, src, n) \
>   __builtin___memcpy_chk (dest, src, n, bos0 (dest))
> ...
> Such built-in functions are provided for @code{memcpy}, @code{mempcpy},
> @code{memmove}, @code{memset}, @code{strcpy}, @code{stpcpy}, @code{strncpy},
> @code{strcat} and @code{strncat}.
> 
> There are also checking built-in functions for formatted output functions.
> @smallexample
> int __builtin___sprintf_chk (char *s, int flag, size_t os, const char *fmt, 
> ...);

Indeed, this __builtin_object_size is useful here, because it works also
on pointers, whereas sizeof() gives the information only on arrays. By combining
both, we can get checking in 5 out of 6 cases in the attached sample.

glibc uses __builtin_object_size for its Fortify support, in the files
  /usr/include/bits/socket2.h
  /usr/include/bits/stdio2.h
  /usr/include/bits/stdlib.h
  /usr/include/bits/string3.h
  /usr/include/bits/unistd.h
  /usr/include/bits/wchar2.h
and conditionalized by
  #if __USE_FORTIFY_LEVEL > 0

There are many functions to which 'char *' pointers are being passed. For
memcpy, memset, memmove, and so on, there is already a size_t argument. The
kinds of errors that the *_chk functions can check for these are only really
dumb beginner mistakes. It's OK for glibc to do that, but I think it's not
necessary for gnulib.

But for functions that don't take a size_t argument, such as
  strcat
  readlink
  realpath
  forkpty
  openpty
  sprintf, u*_sprintf
  vsprintf, u*_vsprintf
  mkdtemp
  mkostemp
  mkstemp
  mkstemps
  mkstemp_safer
  mkostemp_safer
  mkostemps_safer
  mkstemps_safer
  inttostr
these *_chk functions can point out some unobvious mistakes.

Among these, gnulib doesn't replace most of them on glibc systems, and we need
the warnings only on glibc systems. So what we need to handle in gnulib is only
essentially
  u*_sprintf
  u*_vsprintf
  mkstemp_safer
  mkostemp_safer
  mkostemps_safer
  mkstemps_safer
  inttostr

Let's start with inttostr. Using the same technique as glibc in its
/usr/include/bits/unistd.h file, I arrive at the attached code, which produces
a compile-time warning when possible and calls the _chk function when it cannot
prove that the bound it sufficient.

It may look complicated, but it boils down to
  - a bit of macrology and inline functions in inttostr.h,
  - a function inttostr_chk that checks the bound,
  - a use of gl_ASM_SYMBOL_PREFIX in the module description.

Should we pursue this path?

Bruno

Attachment: foo.c
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]