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 _______________________________________________ dm-crypt mailing list dm-crypt@xxxxxxxx http://www.saout.de/mailman/listinfo/dm-crypt
