[PATCH] memset 0 in obscure is optimized away by compiler

Tito farmatito at tiscali.it
Wed Apr 16 17:49:15 UTC 2014


On Wednesday 16 April 2014 19:28:27 Rich Felker wrote:
> On Wed, Apr 16, 2014 at 07:14:05PM +0200, Harald Becker wrote:
> > Hi Tito !
> > 
> > >I've tried to find out if memset is really optimized away in
> > >this case with some test code that I've compiled with :
> > 
> > What is wrong with optimization of code, e.g. replacing call to
> > memset by a quick loop which does same thing even faster than a
> > function call? ... beside that such optimization needs normally a
> > bit more of space it is faster and does same thing as original
> > function call. Try using size optimization (-Os) to avoid
> > replacing function calls by inline replacements.
> 
> The quick loop will equally be optimized away, as neither it nor the
> memset have any observable effect.
> 
> Rich
> 

Hi,
in this case the problem could affect other bb code where nuke_str is used.
Is there a different working solution that could be used?

On https://www.securecoding.cert.org/confluence/display/seccode/MSC06-C.+Beware+of+compiler+optimizations
I read:

"Compliant Solution

This compliant solution uses the volatile type qualifier to inform the compiler that the
memory should be overwritten and that the call to the memset_s() function should not
be optimized out. Unfortunately, this compliant solution may not be as efficient as possible
because of the nature of the volatile type qualifier preventing the compiler from optimizing
the code at all. Typically, some compilers are smart enough to replace calls to memset()
with equivalent assembly instructions that are much more efficient than the memset()
implementation. Implementing a memset_s() function as shown in the example may
prevent the compiler from using the optimal assembly instructions and can result in less efficient code.
Check compiler documentation and the assembly output from the compiler."

/* memset_s.c */
errno_t memset_s(void *v, rsize_t smax, int c, rsize_t n) {
  if (v == NULL) return EINVAL;
  if (smax > RSIZE_MAX) return EINVAL;
  if (n > smax) return EINVAL;
 
  volatile unsigned char *p = v;
  while (smax-- && n--) {
    *p++ = c;
  }
 
  return 0;
}
 
/* getPassword.c */
extern errno_t memset_s(void *v, rsize_t smax, int c, rsize_t n);
 
void getPassword(void) {
  char pwd[64];
 
  if (retrievePassword(pwd, sizeof(pwd))) {
     /* Checking of password, secure operations, etc. */
  }
  if (memset_s(pwd, sizeof(pwd), 0, sizeof(pwd)) != 0) {
    /* Handle error */
  }
}

"This is the preferred solution for C because C11 introduces a memset_s function
with this signature. See the C11-compliant solution for more information.

However, note that both calling functions and accessing volatile-qualified objects can
still be optimized out (while maintaining strict conformance to the standard), so
without a C-conforming implementation, __this compliant solution still might not work in some cases__.

Compliant Solution (C11)

The C Standard includes a memset_s function. Subclause K.3.7.4.1, paragraph 4 [ISO/IEC 9899:2011], states:

    Unlike memset, any call to the memset_s function shall be evaluated strictly according to the rules
of the abstract machine as described in (5.1.2.3). That is, any call to the memset_s
 function shall assume that the memory indicated by s and n may be accessible in
 the future and thus must contain the values indicated by c."

Ciao,
Tito



More information about the busybox mailing list