Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: binary MIME-Version: 1.0 X-Mailer: MIME-tools 5.427 (Entity 5.427) X-RT-Original-Encoding: iso-8859-1 Content-Length: 6084 From krb5-bugs-incoming-bounces@PCH.mit.edu Thu Mar 8 12:05:31 2012 Return-Path: Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (Postfix) with ESMTP id B93EC3DFCD; Thu, 8 Mar 2012 12:05:30 -0500 (EST) Received: from pch.mit.edu (pch.mit.edu [127.0.0.1]) by pch.mit.edu (8.13.6/8.12.8) with ESMTP id q28H5UHf031502; Thu, 8 Mar 2012 12:05:30 -0500 Received: from mailhub-dmz-1.mit.edu (MAILHUB-DMZ-1.MIT.EDU [18.9.21.41]) by pch.mit.edu (8.13.6/8.12.8) with ESMTP id q28Ce1RQ017336 for ; Thu, 8 Mar 2012 07:40:02 -0500 Received: from dmz-mailsec-scanner-6.mit.edu (DMZ-MAILSEC-SCANNER-6.MIT.EDU [18.7.68.35]) by mailhub-dmz-1.mit.edu (8.13.8/8.9.2) with ESMTP id q28CctS7018497 for ; Thu, 8 Mar 2012 07:40:01 -0500 Message-Id: <201203081240.q28CctS7018497@mailhub-dmz-1.mit.edu> X-AuditID: 12074423-b7f9c6d0000008c3-c3-4f58a8a01e01 Authentication-Results: symauth.service.identifier Received: from mx01.meterriblecrew.net (cache.dhh-3.de [62.89.186.8]) by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP id AC.00.02243.0A8A85F4; Thu, 8 Mar 2012 07:40:01 -0500 (EST) Date: Thu, 08 Mar 2012 07:40:00 -0500 Received: (qmail 20669 invoked from network); 8 Mar 2012 12:33:17 -0000 Received: from unknown (HELO localhost) (aw@127.0.0.1) by mx01.instandbesetzt.net with ESMTPA; 8 Mar 2012 12:33:17 -0000 To: krb5-bugs@mit.edu Subject: Issue when disabling assert(3)ions From: aw-devel@meterriblecrew.net X-send-pr-version: 3.99 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrEIsWRWlGSWpSXmKPExsViF7mLQ3fhigh/gwOb2SwaHh5nd2D0aDpz lDmAMYrLJiU1J7MstUjfLoEr4/mO68wFx6Qqti7byNbAuFaki5GTQ0LAROJJyzcWEJtRwEhi 97lXrBBxMYkL99azdTFycQgJnGOU2HtxF1MXIwcHi4CqxLE2ZpAaIQF3iRPHv7NA2NUSu/5M A+sVERCVePn3GFhcWEBb4t2qtWA2m4CixKlbv6F6VSXunvrFDmIzC7BI/HmzgQVir7jEju2n 2Scw8i5gZFjFKJuSW6Wbm5iZU5yarFucnJiXl1qka6aXm1mil5pSuokRGABC7C7KOxj/HFQ6 xCjAwajEw5s1LdxfiDWxrLgy9xCjJAeTkiivNDB8hPiS8lMqMxKLM+KLSnNSiw8xSnAwK4nw dncA5XhTEiurUovyYVLSHCxK4rwaWu/8hATSE0tSs1NTC1KLYLJMHOyHGGU4OJQkeJVBJgsW paanVqRl5pQgq+EEEVwga3iA1vCAFPIWFyTmFmemQxSdYlSUEuf9tBwoIQCSyCjNgxsAi9pL jLJSwryMDAwMQjxAFwA9jir/ilEc6GlhXkmQ8TyZeSVw018BLWYCWpwpEw6yuCQRISXVwOg8 9WnXnE3ml8oEIj2SO58kXMv48N7v+dYL6+fb5SVG7mVlcfZbXsV/2GCDV/+cE55eSu1OnjKv MnOzLP/UHTn8zK7tXNjFilVu6475q7E+yj95cXsmj63sXX6HNO0zwZF6Z9vz5LQ95myotHoW sjNv2Q7z6fe3mEy09mu9f8LfUMdy+vJFU5VYijMSDbWYi4oTAZB2uYHVAgAA X-Mailman-Approved-At: Thu, 08 Mar 2012 12:05:15 -0500 X-BeenThere: krb5-bugs-incoming@mailman.mit.edu X-Mailman-Version: 2.1.6 Precedence: list Reply-To: aw-devel@meterriblecrew.net Sender: krb5-bugs-incoming-bounces@PCH.mit.edu Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu >Submitter-Id: net >Originator: Andreas Wiese >Organization: Meterriblecrew.NET >Confidential: no >Synopsis: Building with CFLAGS+=-DNDEBUG build things >Severity: serious >Priority: medium >Category: krb5-libs >Class: sw-bug >Release: 1.10 >Environment: System: Linux incendiary.meterriblecrew.net 3.3.0-rc6 #1 SMP PREEMPT Sun Mar 4 18:46:34 CET 2012 x86_64 Intel(R) Core(TM)2 Duo CPU L7700 @ 1.80GHz GenuineIntel GNU/Linux >Description: I tried building krb5-1.10 with CFLAGS="… -DNDEBUG" today. This breaks compilation, since »uninitialized variables« are encountered by gcc and since build-system uses -Werror gcc bails out. Hunting the problem down in src/lib/crypto/krb/cf2.c I found out that gcc is right in case assertions are disabled, since the initialization of out_enctype happens inside the assert()-statement (line 110): 90 krb5_error_code KRB5_CALLCONV 91 krb5_c_fx_cf2_simple(krb5_context context, 92 krb5_keyblock *k1, const char *pepper1, 93 krb5_keyblock *k2, const char *pepper2, 94 krb5_keyblock **out) 95 { 96 const struct krb5_keytypes *out_enctype; 97 size_t keybytes, keylength, i; 98 char *prf1 = NULL, *prf2 = NULL; 99 krb5_data keydata; 100 krb5_enctype out_enctype_num; 101 krb5_error_code retval = 0; 102 krb5_keyblock *out_key = NULL; 103 104 if (k1 == NULL || !krb5_c_valid_enctype(k1->enctype)) 105 return KRB5_BAD_ENCTYPE; 106 if (k2 == NULL || !krb5_c_valid_enctype(k2->enctype)) 107 return KRB5_BAD_ENCTYPE; 108 out_enctype_num = k1->enctype; 109 assert(out != NULL); 110 assert((out_enctype = find_enctype(out_enctype_num)) != NULL); 111 if (out_enctype->prf == NULL) { 112 if (context) 113 krb5int_set_error(&(context->err), KRB5_CRYPTO_INTERNAL, 114 _("Enctype %d has no PRF"), out_enctype_num); 115 return KRB5_CRYPTO_INTERNAL; 116 } I consider this a bug. Assertions aren't meant to contain program logic, but to help the _programmer_ find bugs. It's fully reasonable having them disabled on production use and even if this bug won't silently be introduced since you're building with -Werror enabled by default, it's not meant to be used like this. >How-To-Repeat: $ make CFLAGS="-DNDEBUG" >Fix: I'm suggesting the following as a fix: --- cf2.c.old 2011-06-10 20:17:37.000000000 +0200 +++ cf2.c 2012-03-08 13:26:06.614544255 +0100 @@ -107,7 +107,8 @@ return KRB5_BAD_ENCTYPE; out_enctype_num = k1->enctype; assert(out != NULL); - assert((out_enctype = find_enctype(out_enctype_num)) != NULL); + out_enctype = find_enctype(out_enctype_num); + assert(out_enctype != NULL); if (out_enctype->prf == NULL) { if (context) krb5int_set_error(&(context->err), KRB5_CRYPTO_INTERNAL, Besides that one should ensure that no program logic happens inside assert(3)s. This is a found-by-luck kind of bug — thanks to gcc. I'm currently not capable of hunting down similiar issues, but »grep -r 'assert(.*[^<>!=]=[^].*)' .« suggests to have a closer look on this (got a hit in src/util/profile/prof_file.c where it doesn't lead to related compile-time-errors).