From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from scc-mailout-kit-02.scc.kit.edu (scc-mailout-kit-02.scc.kit.edu [129.13.231.82]) by fantadrom.bsd.lv (OpenSMTPD) with ESMTP id bdf48534 for ; Sat, 16 Jul 2016 15:44:26 -0500 (EST) Received: from asta-nat.asta.uni-karlsruhe.de ([172.22.63.82] helo=hekate.usta.de) by scc-mailout-kit-02.scc.kit.edu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (envelope-from ) id 1bOWRj-0005jh-RT; Sat, 16 Jul 2016 22:44:25 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1bOWRf-00073K-LV; Sat, 16 Jul 2016 22:44:19 +0200 Received: from athene.usta.de ([172.24.96.10]) by donnerwolke.usta.de with esmtp (Exim 4.84_2) (envelope-from ) id 1bOWRf-00048O-Hl; Sat, 16 Jul 2016 22:44:19 +0200 Received: from localhost (athene.usta.de [local]) by athene.usta.de (OpenSMTPD) with ESMTPA id 22f438b9; Sat, 16 Jul 2016 22:44:19 +0200 (CEST) Date: Sat, 16 Jul 2016 22:44:19 +0200 From: Ingo Schwarze To: Joerg Sonnenberger Cc: tech@mdocml.bsd.lv Subject: Re: mdocml: To remove the const qualifier from a pointer to an object Message-ID: <20160716204419.GA5462@athene.usta.de> References: <15489725859697430873.enqueue@fantadrom.bsd.lv> <20160715213103.GA7335@britannica.bec.de> <20160715220846.GA13283@athene.usta.de> <20160716163740.GA32161@britannica.bec.de> X-Mailinglist: mdocml-tech Reply-To: tech@mdocml.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160716163740.GA32161@britannica.bec.de> User-Agent: Mutt/1.6.2 (2016-07-01) Hi Joerg, Joerg Sonnenberger wrote on Sat, Jul 16, 2016 at 06:37:40PM +0200: > On Sat, Jul 16, 2016 at 12:08:46AM +0200, Ingo Schwarze wrote: >> Joerg Sonnenberger wrote: >>> (1) It makes auditing easier by giving something simple to look for. >> It makes auditing harder due to obfuscation. What a plain cast >> does is obvious on first sight. A macro needs to be looked up when >> auditing the code to understand what it does, and due to the >> unnecessary quadruple cast const char **" -> const void * -> >> intptr_t -> void * -> char * const * instead of one single cast >> it makes auditing even harder. > This is plainly wrong. A type cast can have a hundred different > meanings. It is far from obvious that it is only meant to remove > a type qualifier. We are talking about code like this: const char *ccp; [...] something involving (char *)ccp; It's completely obvious not only what the intention is, but in addition to that, the code exactly fulfills the intention: It changes nothing but removing the qualifier. > If you can't derive the intention from a macro called UNCONST... As shown above, the intention *is* obvious even without having a macro called UNCONST. Even worse than that, the name UNCONST is an outrageous lie. The macro does much more than merely removing the const qualifier. Consider const struct timeval *tv; struct timespec *ts; [...] ts = UNCONST(tv); Such code can have disastrous consequences because on some platforms, suseconds_t may be narrower than long, so here we have a potential write buffer overflow. But due to the excessively powerful UNCONST macro, the compiler will not warn, whereas ts = (struct timeval *)tv; is likely to cause a warning. The intent of the latter is just as obvious, if not more obvious, and it is less dangerous because it only does what is intended to do and nothing unintended besides. >> I see no point in specifically auditing for casts removing const. > Given that const memory can actually be read-only, removing the logical > property is a clear sign of either a misdesigned interface, a workaround > for a language defect or a plain bug to be waiting. These bugs are usually relatively benign, though. You write to immutable memory? SIGSEGF. Easy to find, easy to fix, rarely exploitable. Besides, such bugs are rare. I have seen lots and lots of buffer overruns and signedness issues, many of them very dangerous, and very few bogus const castaways, if any. > It is no less a warning sign than use of strcpy.. Bad comparison: That usually leads to write buffer overruns and no segfault - hard to find, often exploitable. Many type conversion issues are actually notorious for being security critical: Signedness, width mismatch, overflows... Bogus loss of const, by constrast, is relatively benign, so UNCONST increases the risk of unintentionally hiding width and signedness issues, just to make something rather harmless stand out prominently. > It is used in production builds by many systems, including FreeBSD and > NetBSD, for a lot of software. Just because you don't do that in > OpenBSD doesn't mean it doesn't create problems for others. The problem is easy to solve, though, by simply adding -Wno-cast-qual to the mandoc Makefile, without disturbing anything in the rest of your tree. I'm not sure how useful it is to continue the discussion. In a nutshell, we disagree regarding a style issue. There are arguments either way; the ones against UNCONST seem stronger to me, and so far, you claim the opposite. Of course, it would be nice if we could find an agreement, but it's not the end of the world if we don't... Yours, Ingo -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv