Andreas F. Borchert
2013-Aug-21 16:32 UTC
[Dovecot] Bug in dovecot 2.2.5: segfault due to bad alignment
Take a look at the sources, hmac.h declares struct hmac_context: struct hmac_context { char ctx[HMAC_MAX_CONTEXT_SIZE]; char ctxo[HMAC_MAX_CONTEXT_SIZE]; const struct hash_method *hash; }; If compiled for a 32 bit virtual address space, this has an alignment requirement of 4 due to the hash pointer. In line 171 of auth-token.c, we have following declaration of ctx as a local variable in auth_token_get(): struct hmac_context ctx; This is put on an address with an alignment requirement of 4. In lines 174 and 175 hmac_init is invoked with hash_method_sha1: hmac_init(&ctx, (const unsigned char*)username, strlen(username), &hash_method_sha1); In hmac.c, lines 43 and following, ctx->ctx with an alignment of 4 is passed to meth->init and meth->loop where meth refers to hash_method_sha1: meth->init(ctx->ctx); meth->loop(ctx->ctx, k_ipad, 64); These functions refer now to sha1_init and sha1_loop where the first parameter is expected to be a pointer to struct sha1_ctxt, a data structure which is declared in sha1.h: struct sha1_ctxt { union { uint8_t b8[20]; uint32_t b32[5]; } h; union { uint8_t b8[8]; uint64_t b64[1]; } c; union { uint8_t b8[64]; uint32_t b32[16]; } m; uint8_t count; }; Here we have with b64 one uint64_t which has on a SPARC platform an alignment requirement of 8. In consequence, struct sha1_ctxt has an alignment requirement of 8. With the invocations of meth->init and meth->loop above we pass a pointer to a data structure of alignment 4 to a function expecting a pointer to a data structure of alignment 8. Chances are that the alignment requirement is not met, causing a segmentation violation. This must be solved by declaring struct hmac_context such that is not just big enough but respects also the highest alignment required for one of the hashing data structures. There are several options to do this: * Beginning with C11, you are free to use an alignment specifier, i.e. add _Alignas ( uint64_t ) (see section 6.7.5 in ISO 9899-2011) * GCC supports alignment attributes, i.e. add __attribute__ ((aligned (8))) or whatever is required instead of 8, see http://gcc.gnu.org/onlinedocs/gcc/Variable-Attributes.html * Do not use a local variable for it, allocate the data structure using malloc instead. If you want to see a live crash, here is the relevant output of gdb that debugs ${prefix}/libexec/dovecot/auth. Program received signal SIGSEGV, Segmentation fault. sha1_loop (ctxt=0xffbff63c, input=0xffbff548, len=64) at sha1.c:224 224 sha1.c: No such file or directory. (gdb) where #0 sha1_loop (ctxt=0xffbff63c, input=0xffbff548, len=64) at sha1.c:224 #1 0xff2e218c in hmac_init (ctx=ctx at entry=0xffbff63c, key=key at entry=0x6a698 "borchert", key_len=8, meth=0x555d0 <hash_method_sha1>) at hmac.c:44 #2 0x00023310 in auth_token_get (service=service at entry=0x6a648 "imap", session_pid=0x56071 "26272", username=0x6a698 "borchert", session_id=0x6a650 "AErv6nbk6gB/AAAB") at auth-token.c:174 #3 0x00021708 in userdb_callback (result=USERDB_RESULT_OK, request=0x6a530) at auth-request-handler.c:668 #4 0x0001f144 in auth_request_userdb_callback (result=<optimized out>, result at entry=USERDB_RESULT_OK, request=request at entry=0x6a530) at auth-request.c:1039 #5 0x000312c8 in prefetch_lookup (auth_request=0x6a530, callback=0x1f058 <auth_request_userdb_callback>) at userdb-prefetch.c:40 #6 0x0001f37c in auth_request_lookup_user (request=0x6a530, callback=callback at entry=0x2150c <userdb_callback>) at auth-request.c:1072 #7 0x00022034 in auth_request_handler_master_request ( handler=<optimized out>, master=master at entry=0x6b120, id=1292369921, client_id=1, params=0x55bfc) at auth-request-handler.c:758 #8 0x0001be98 in master_input_request (args=<optimized out>, conn=0x6b120) at auth-master-connection.c:127 #9 auth_master_input_line (line=<optimized out>, conn=0x6b120) at auth-master-connection.c:598 #10 master_input (conn=0x6b120) at auth-master-connection.c:653 #11 0xff2ecca4 in io_loop_call_io (io=io at entry=0x6b398) at ioloop.c:387 #12 0xff2ed604 in io_loop_handler_run (ioloop=ioloop at entry=0x5e5e8) at ioloop-poll.c:211 #13 0xff2ec7a8 in io_loop_run (ioloop=0x5e5e8) at ioloop.c:406 #14 0xff29ad7c in master_service_run (service=0x5e128, callback=0x27b40 <client_connected>) at master-service.c:566 #15 0x0001852c in main (argc=1, argv=0xffbffd54) at main.c:393 (gdb) print ctxt $1 = (struct sha1_ctxt *) 0xffbff63c As you can see, ctxt is on a 4-byte boundary, not on an 8-byte boundary. The crash happens at sha1.c:224 where the 8-byte-alignment is indeed mandatory on a SPARC architecture: ctxt->c.b64[0] += copysiz * 8; The environment is Solaris 10 on SPARCv9. The sources have been compiled using gcc 4.8.0 for 32 bit. Andreas.
Timo Sirainen
2013-Aug-28 22:44 UTC
[Dovecot] Bug in dovecot 2.2.5: segfault due to bad alignment
Attached patch fixes this? If yes, I'll commit it. -------------- next part -------------- A non-text attachment was scrubbed... Name: diff Type: application/octet-stream Size: 2705 bytes Desc: not available URL: <http://dovecot.org/pipermail/dovecot/attachments/20130829/49e231b6/attachment-0001.obj> -------------- next part -------------- On 21.8.2013, at 19.32, Andreas F. Borchert <dovecot at andreas-borchert.de> wrote:> Take a look at the sources, hmac.h declares struct hmac_context: > > struct hmac_context { > char ctx[HMAC_MAX_CONTEXT_SIZE]; > char ctxo[HMAC_MAX_CONTEXT_SIZE]; > const struct hash_method *hash; > }; > > If compiled for a 32 bit virtual address space, this has an alignment > requirement of 4 due to the hash pointer. > > In line 171 of auth-token.c, we have following declaration of ctx > as a local variable in auth_token_get(): > > struct hmac_context ctx; > > This is put on an address with an alignment requirement of 4. In > lines 174 and 175 hmac_init is invoked with hash_method_sha1: > > hmac_init(&ctx, (const unsigned char*)username, strlen(username), > &hash_method_sha1); > > In hmac.c, lines 43 and following, ctx->ctx with an alignment of > 4 is passed to meth->init and meth->loop where meth refers to > hash_method_sha1: > > meth->init(ctx->ctx); > meth->loop(ctx->ctx, k_ipad, 64); > > These functions refer now to sha1_init and sha1_loop where the > first parameter is expected to be a pointer to struct sha1_ctxt, > a data structure which is declared in sha1.h: > > struct sha1_ctxt { > union { > uint8_t b8[20]; > uint32_t b32[5]; > } h; > union { > uint8_t b8[8]; > uint64_t b64[1]; > } c; > union { > uint8_t b8[64]; > uint32_t b32[16]; > } m; > uint8_t count; > }; > > Here we have with b64 one uint64_t which has on a SPARC platform > an alignment requirement of 8. In consequence, struct sha1_ctxt > has an alignment requirement of 8. With the invocations of > meth->init and meth->loop above we pass a pointer to a data > structure of alignment 4 to a function expecting a pointer to > a data structure of alignment 8. Chances are that the alignment > requirement is not met, causing a segmentation violation. > > This must be solved by declaring struct hmac_context such that > is not just big enough but respects also the highest alignment > required for one of the hashing data structures. > > There are several options to do this: > > * Beginning with C11, you are free to use an alignment specifier, > i.e. add _Alignas ( uint64_t ) > (see section 6.7.5 in ISO 9899-2011) > > * GCC supports alignment attributes, > i.e. add __attribute__ ((aligned (8))) > or whatever is required instead of 8, > see http://gcc.gnu.org/onlinedocs/gcc/Variable-Attributes.html > > * Do not use a local variable for it, allocate the data structure > using malloc instead. > > If you want to see a live crash, here is the relevant output of gdb > that debugs ${prefix}/libexec/dovecot/auth. > > Program received signal SIGSEGV, Segmentation fault. > sha1_loop (ctxt=0xffbff63c, input=0xffbff548, len=64) at sha1.c:224 > 224 sha1.c: No such file or directory. > (gdb) where > #0 sha1_loop (ctxt=0xffbff63c, input=0xffbff548, len=64) at sha1.c:224 > #1 0xff2e218c in hmac_init (ctx=ctx at entry=0xffbff63c, > key=key at entry=0x6a698 "borchert", key_len=8, > meth=0x555d0 <hash_method_sha1>) at hmac.c:44 > #2 0x00023310 in auth_token_get (service=service at entry=0x6a648 "imap", > session_pid=0x56071 "26272", username=0x6a698 "borchert", > session_id=0x6a650 "AErv6nbk6gB/AAAB") at auth-token.c:174 > #3 0x00021708 in userdb_callback (result=USERDB_RESULT_OK, request=0x6a530) > at auth-request-handler.c:668 > #4 0x0001f144 in auth_request_userdb_callback (result=<optimized out>, > result at entry=USERDB_RESULT_OK, request=request at entry=0x6a530) > at auth-request.c:1039 > #5 0x000312c8 in prefetch_lookup (auth_request=0x6a530, > callback=0x1f058 <auth_request_userdb_callback>) at userdb-prefetch.c:40 > #6 0x0001f37c in auth_request_lookup_user (request=0x6a530, > callback=callback at entry=0x2150c <userdb_callback>) at auth-request.c:1072 > #7 0x00022034 in auth_request_handler_master_request ( > handler=<optimized out>, master=master at entry=0x6b120, id=1292369921, > client_id=1, params=0x55bfc) at auth-request-handler.c:758 > #8 0x0001be98 in master_input_request (args=<optimized out>, conn=0x6b120) > at auth-master-connection.c:127 > #9 auth_master_input_line (line=<optimized out>, conn=0x6b120) > at auth-master-connection.c:598 > #10 master_input (conn=0x6b120) at auth-master-connection.c:653 > #11 0xff2ecca4 in io_loop_call_io (io=io at entry=0x6b398) at ioloop.c:387 > #12 0xff2ed604 in io_loop_handler_run (ioloop=ioloop at entry=0x5e5e8) > at ioloop-poll.c:211 > #13 0xff2ec7a8 in io_loop_run (ioloop=0x5e5e8) at ioloop.c:406 > #14 0xff29ad7c in master_service_run (service=0x5e128, > callback=0x27b40 <client_connected>) at master-service.c:566 > #15 0x0001852c in main (argc=1, argv=0xffbffd54) at main.c:393 > (gdb) print ctxt > $1 = (struct sha1_ctxt *) 0xffbff63c > > As you can see, ctxt is on a 4-byte boundary, not on an 8-byte boundary. > The crash happens at sha1.c:224 where the 8-byte-alignment is indeed > mandatory on a SPARC architecture: > > ctxt->c.b64[0] += copysiz * 8; > > The environment is Solaris 10 on SPARCv9. The sources have been compiled > using gcc 4.8.0 for 32 bit. > > Andreas. >