[dm-crypt] crypsetup segfaulting during luksFormat

Sven Eschenberg sven at whgl.uni-frankfurt.de
Thu Jul 8 16:29:44 CEST 2010


Hi Milan,

Even worse, actually byte swappig is done to store the int in a big
endian manner, unfortunately, since it is done wrong, on big endian
systems all block indeces would be zero and they are part of the Derived
Key. I wonder if this has a security impact as in quality of derived key
on big endian systems.

On Thu, 2010-07-08 at 16:16 +0200, Milan Broz wrote:
> On 07/08/2010 03:37 PM, Sven Eschenberg wrote:
> > Just for the record:
> > 
> > The crash happens with other gcc versions as well. As the gentoo bug
> > report suggests, it seems to be a problem when the executeable is linked
> > statically on hardened profiles.
> > And yes, in my case compiling it dynamically resolves the segfault
> > aswell.
> 
> I am compiling static version quite often, so hardened profile probably uses
> some not common compiled switch for static version.

Yes, a hardened glibc behaves quite differently, esp. layout of
allocated memory and maybe(?) this has influence on an alloca as well,
who knows.

> 
> > In the src the following variables are used in the handler:
> > 
> > static volatile uint64_t __PBKDF2_global_j = 0;
> > static volatile uint64_t __PBKDF2_performance = 0;
> > 
> > Since they are used in the sighandler, they would better not just be
> > volatile but sig_atomic_t, to avoid possible races.
> 
> yes
> 
> > But this should not have any influence on the segfault as far as I can
> > tell.
> > 
> > Oh, and better use sigaction() instead of signal().
> 
> why? should be no problem here. (that code is ugly anyway, I just polished
> it some time ago when replacing pbkdf2 with gcrypt version...)
> 

well, okay, cryptsetup is linux specific, so, yes, we can agree that
portability is meaningless, but it is stated:

The only portable use of signal() is to set a signal's disposition to
SIG_DFL or SIG_IGN. The semantics when using signal() to establish a
signal handler vary across systems (and POSIX.1 explicitly permits this
variation); do not use it for this purpose.

So, it's more a question of style in our case.
> 
> > I think I possibly found the problem:
> > 
> > In static int pkcs5_pbkdf2() in pbkdf.c:
> > 
> > size_t tmplen = Slen + 4;
> > tmp = alloca(tmplen); // allocate Slen+4 bytes on the stack ...
> 
> so problem is implicit type cast? interesting...
> 
> seems to be some relict from former implementation, I am always
> trying to avoid alloca() in code... :)
> (wonder if valgrind find that)
> 

Yes, funny thing is, this is a typical usage (try making it endian
safe), and I've seen the exactly same mistake (diffferent lengths of
types and thus running out of bound) over and over again. Last time it
was pretty much the same thing, code worked (luckily) and when it moved
to another OS with other allocation - segmentation fault.
> 
> Thanks!
> Milan

You are welcome, I did not modify the lines yet for testing, it just
popped up my eyes when I looked at the code there.

-Sven




More information about the dm-crypt mailing list