valgrind-users

[Valgrind-users] Getting a spurious invalid-write error with a program that uses swapcontext

[Zack W. <za...@pa...>]

The test program at the end of this message is cut down from a real
program that uses swapcontext and friends (from ucontext.h) to run a
computation on sensitive data and then erase the stack used by that
computation.  The memset at the end of call_worker provokes an
invalid-write error from memcheck:

==12261== Memcheck, a memory error detector
==12261== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==12261== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==12261== Command: ./vg_test_2
==12261==
sizeof(data)        = 17328
offsetof(inner_ctx) = 16384
34 34
==12261== Invalid write of size 8
==12261==    at 0x4C32547: memset (vg_replace_strmem.c:1234)
==12261==    by 0x1089E6: call_worker.constprop.0 (vg_test_2.c:42)
==12261==    by 0x10874E: main (vg_test_2.c:62)
==12261==  Address 0x30cfe0 is 16224 bytes inside data symbol "ss.3473"
==12261==
00 00
==12261==
==12261== HEAP SUMMARY:
==12261==     in use at exit: 0 bytes in 0 blocks
==12261==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==12261==
==12261== All heap blocks were freed -- no leaks are possible
==12261==
==12261== For counts of detected and suppressed errors, rerun with: -v
==12261== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

"16224 bytes inside data symbol ss.3473" is 160 bytes from the high
end of the inner stack -- it's plausible that about this much space
would be used on the inner stack, on this architecture, by using
makecontext and swapcontext to call a no-op function.  If you compile
the test program with -DDONT_CALL_SWAPCONTEXT, the error goes away, so
this is definitely provoked by whatever swapcontext does internally.
Note that the program already has VALGRIND_STACK_REGISTER/DEREGISTER
annotations.

I can work around this by forcibly adjusting valgrind's state for the
"data" object (compile with -DDO_MAKE_MEM_WRITABLE to see) but I would
like to understand *why* this memory has become unwritable, why the
STACK_REGISTER/DEREGISTER was insufficient, and whether there's a more
appropriate work-around.

Thanks,
zw

#include <stdalign.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ucontext.h>
#include <valgrind/memcheck.h>

struct data
{
  char alignas(max_align_t) inner_stack[16384];
  ucontext_t inner_ctx;
};

static void
worker(void)
{
}

static void
call_worker(struct data *ip)
{
  ucontext_t outer_ctx;
  unsigned vg_stackid;
  if (getcontext(&ip->inner_ctx))
    abort();

  vg_stackid = VALGRIND_STACK_REGISTER
    (ip->inner_stack, ip->inner_stack + sizeof ip->inner_stack);

  ip->inner_ctx.uc_stack.ss_sp   = ip->inner_stack;
  ip->inner_ctx.uc_stack.ss_size = sizeof ip->inner_stack;
  ip->inner_ctx.uc_link          = &outer_ctx;
  makecontext(&ip->inner_ctx, worker, 0);

#ifndef DONT_CALL_SWAPCONTEXT
  swapcontext(&outer_ctx, &ip->inner_ctx);
#endif

  VALGRIND_STACK_DEREGISTER(vg_stackid);

#ifdef DO_MAKE_MEM_WRITABLE
  VALGRIND_MAKE_MEM_UNDEFINED(ip, sizeof *ip);
#endif

  memset(ip, 0, sizeof *ip);
}

#define ZX(x) ((unsigned int)(unsigned char)(x))

int
main(void)
{
  printf("sizeof(data)        = %zu\n"
         "offsetof(inner_ctx) = %zu\n",
         sizeof(struct data),
         offsetof(struct data, inner_ctx));

  static struct data ss;
  memset(&ss, 0x34, sizeof ss);

  printf("%02x %02x\n",
         ZX(((char *)&ss)[0]),
         ZX(((char *)&ss)[sizeof ss - 1]));

  call_worker(&ss);

  printf("%02x %02x\n",
         ZX(((char *)&ss)[0]),
         ZX(((char *)&ss)[sizeof ss - 1]));

  return 0;
}

Re: [Valgrind-users] Getting a spurious invalid-write error with a program that uses swapcontext

[Philippe W. <phi...@sk...>]

On Thu, 2017-10-19 at 12:06 -0400, Zack Weinberg wrote:
> I can work around this by forcibly adjusting valgrind's state for the
> "data" object (compile with -DDO_MAKE_MEM_WRITABLE to see) but I
> would
> like to understand *why* this memory has become unwritable, why the
> STACK_REGISTER/DEREGISTER was insufficient, and whether there's a
> more
> appropriate work-around.

STACK_REGISTER (STACK_DEREGISTER) is just marking a piece
of memory as being a stack (not being a stack anymore).
This is a.o. used to help unwind logic, provide better description
of an address in error messages, etc ...

These do not change the Addressable or Validity-bits of the
 piece of memory.
So, after a piece of memory has been used as a stack,
depending on how much of this stack was used, the 'normal'
valgrind memcheck code has marked some memory unaccessible
Typically, when a function returns, the space that was used
by the frame of the returning function is marked as unaccessible.

When you deregister the stack, these V-bits/A-bits are not 
changed, and so part of this memory is now marked as not addressable.

IMO, before switching to a new stack, it would be better to
mark the stack as not addressable (maybe a small part of the
stack must be kept addressable, to just allow switching to this
stack).
Marking the stack as unaddressable would help memcheck to detect e.g.
a dereference to a 'not valid' part of the stack.

And when a stack is not used anymore, it might be needed
(like in your case) to mark the memory as addressable again.

So, I think that what you see is all normal :
STACK_REGISTER (STACK_DEREGISTER) are low level features,
they do not do the whole valgrind magic that might be needed
e.g. for your memset after usage.
And I think that your usage of VALGRIND_MAKE_MEM_UNDEFINED
is the correct thing to do.

Philippe