From debbugs-submit-bounces@debbugs.gnu.org Mon May 14 08:21:53 2012 Received: (at submit) by debbugs.gnu.org; 14 May 2012 12:21:54 +0000 Received: from localhost ([127.0.0.1]:52251 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STuHg-0005QZ-SN for submit@debbugs.gnu.org; Mon, 14 May 2012 08:21:53 -0400 Received: from eggs.gnu.org ([208.118.235.92]:44981) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STuHc-0005QD-FL for submit@debbugs.gnu.org; Mon, 14 May 2012 08:21:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1STuH6-00017F-Qc for submit@debbugs.gnu.org; Mon, 14 May 2012 08:21:21 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.2 Received: from lists.gnu.org ([208.118.235.17]:45682) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STuH6-00017B-N3 for submit@debbugs.gnu.org; Mon, 14 May 2012 08:21:16 -0400 Received: from eggs.gnu.org ([208.118.235.92]:33686) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STuH1-0000rg-Ds for bug-coreutils@gnu.org; Mon, 14 May 2012 08:21:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1STuGv-00015z-3c for bug-coreutils@gnu.org; Mon, 14 May 2012 08:21:10 -0400 Received: from rcsinet15.oracle.com ([148.87.113.117]:29374) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STuGu-00015e-Qw for bug-coreutils@gnu.org; Mon, 14 May 2012 08:21:05 -0400 Received: from acsinet21.oracle.com (acsinet21.oracle.com [141.146.126.237]) by rcsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q4ECL0ZH009944 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 14 May 2012 12:21:01 GMT Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q4ECL0hY014903 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 14 May 2012 12:21:00 GMT Received: from abhmt108.oracle.com (abhmt108.oracle.com [141.146.116.60]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q4ECKxTZ021904 for ; Mon, 14 May 2012 07:20:59 -0500 Received: from [10.132.144.96] (/10.132.144.96) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 14 May 2012 05:20:59 -0700 Message-ID: <4FB0F9A5.7020306@oracle.com> Date: Mon, 14 May 2012 05:25:09 -0700 From: Rich Burridge User-Agent: Mozilla/5.0 (X11; SunOS i86pc; rv:10.0.3) Gecko/20120424 Thunderbird/10.0.3 MIME-Version: 1.0 To: bug-coreutils@gnu.org Subject: Parfait problems with GNU coreutils Content-Type: multipart/mixed; boundary="------------020109050107040102010806" X-Source-IP: acsinet21.oracle.com [141.146.126.237] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 1) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 208.118.235.17 X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) This is a multi-part message in MIME format. --------------020109050107040102010806 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi, I've been running parfait [1] on GNU coreutils and it found the following problems: Error: Buffer overrun Buffer overflow (CWE 120): In array dereference of word_limit[-1] with index '-1' Array size is 1000 elements (of 28 bytes each), -1 is -1 at line 590 of components/coreutils/coreutils-8.5/src/fmt.c in function 'get_paragraph'. Read outside array bounds (CWE 125): In array dereference of word_limit[-1] with index '-1' Array size is 1000 elements (of 28 bytes each), -1 is -1 at line 590 of components/coreutils/coreutils-8.5/src/fmt.c in function 'get_paragraph'. Error: Null pointer dereference (CWE 476) Read from null pointer 's' at line 3389 of components/coreutils/coreutils-8.5/src/sort.c in function 'main'. Function 'parse_field_count' may return constant 'NULL' at line 3130, called at line 3387. Null pointer introduced at line 3130 in function 'parse_field_count'. Error: Null pointer dereference (CWE 476) Read from null pointer 'bitsp' at line 1546 of components/coreutils/coreutils-8.5/src/stty.c in function 'display_changed'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1544. Null pointer introduced at line 1453 in function 'mode_type_flag'. Error: Null pointer dereference (CWE 476) Read from null pointer 'bitsp' at line 1623 of components/coreutils/coreutils-8.5/src/stty.c in function 'display_all'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1621. Null pointer introduced at line 1453 in function 'mode_type_flag'. Error: Null pointer dereference (CWE 476) Read from null pointer 'bitsp' at line 1838 of components/coreutils/coreutils-8.5/src/stty.c in function 'sane_mode'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1837. Null pointer introduced at line 1453 in function 'mode_type_flag'. at line 1843 of components/coreutils/coreutils-8.5/src/stty.c in function 'sane_mode'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1842. Null pointer introduced at line 1453 in function 'mode_type_flag'. Error: Null pointer dereference (CWE 476) Write to null pointer 'bitsp' at line 1838 of components/coreutils/coreutils-8.5/src/stty.c in function 'sane_mode'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1837. Null pointer introduced at line 1453 in function 'mode_type_flag'. at line 1843 of components/coreutils/coreutils-8.5/src/stty.c in function 'sane_mode'. Function 'mode_type_flag' may return constant 'NULL' at line 1453, called at line 1842. Null pointer introduced at line 1453 in function 'mode_type_flag'. I've attached a patch that we are applying to the code to fix these problems. Here's the evaluation of why the changes have been made: There are three different types of Parfait errors here: 1/ In fmt.c, in the get_paragraph() routine, Parfait thinks that there is a potential problem of a negative index into the 'word' array. But that situation is impossible. Earlier in the routine, get_line() is called, and this has to have incremented word_limit. The solution was to add a Parfait style comment before the offending line of code to shut parfait up. 2/ In sort.c, there is an occurrence in the main() routine where 's' was being dereferenced, but it could have been NULL. A check was added to only do the dereferencing if that wasn't the case. It was noticed that there was already similar code in the same routine, so this seems a reasonable solution. 3/ In stty.c, there were two occurrences where bitsp was being dereferenced that could have been a NULL pointer. In each case, a check was added to only do the dereferencing if that wasn't the case. It was noticed that there was already similar code in the sane_mode() routine, so this seems a reasonable solution. These changes make us "parfait clean". I realize there is a later version of GNU coreutils, but this is the version that currently ships with Solaris 11. If these problems still exist in the current version, (and assuming you think these changes are good), hopefully you'll like to make similar changes in the next version of GNU coreutils. Thanks. [1] http://labs.oracle.com/projects/parfait/ --------------020109050107040102010806 Content-Type: text/plain; name="parfait-fix.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="parfait-fix.patch" --- coreutils-8.5/src/fmt.c.orig 2012-04-30 10:37:50.082717390 -0700 +++ coreutils-8.5/src/fmt.c 2012-05-01 11:28:00.745415600 -0700 @@ -587,6 +587,7 @@ while (same_para (c) && in_column == other_indent) c = get_line (f, c); } + /* Parfait_ALLOW buffer-overflow word_limit will be incremented by get_line */ (word_limit - 1)->period = (word_limit - 1)->final = true; next_char = c; return true; --- coreutils-8.5/src/sort.c.orig 2012-04-30 10:19:57.802726721 -0700 +++ coreutils-8.5/src/sort.c 2012-04-30 10:20:49.681778327 -0700 @@ -3386,7 +3386,7 @@ char const *optarg1 = argv[optind++]; s = parse_field_count (optarg1 + 1, &key->eword, N_("invalid number after `-'")); - if (*s == '.') + if (s && *s == '.') s = parse_field_count (s + 1, &key->echar, N_("invalid number after `.'")); if (!key->echar && key->eword) --- coreutils-8.5/src/stty.c.orig 2012-04-30 10:29:20.174932138 -0700 +++ coreutils-8.5/src/stty.c 2012-04-30 11:20:00.683384538 -0700 @@ -1543,7 +1543,7 @@ bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; - if ((*bitsp & mask) == mode_info[i].bits) + if (bitsp && ((*bitsp & mask) == mode_info[i].bits)) { if (mode_info[i].flags & SANE_UNSET) { @@ -1620,7 +1620,7 @@ bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; - if ((*bitsp & mask) == mode_info[i].bits) + if (bitsp && ((*bitsp & mask) == mode_info[i].bits)) wrapf ("%s", mode_info[i].name); else if (mode_info[i].flags & REV) wrapf ("-%s", mode_info[i].name); @@ -1835,12 +1835,14 @@ if (mode_info[i].flags & SANE_SET) { bitsp = mode_type_flag (mode_info[i].type, mode); - *bitsp = (*bitsp & ~mode_info[i].mask) | mode_info[i].bits; + if (bitsp) + *bitsp = (*bitsp & ~mode_info[i].mask) | mode_info[i].bits; } else if (mode_info[i].flags & SANE_UNSET) { bitsp = mode_type_flag (mode_info[i].type, mode); - *bitsp = *bitsp & ~mode_info[i].mask & ~mode_info[i].bits; + if (bitsp) + *bitsp = *bitsp & ~mode_info[i].mask & ~mode_info[i].bits; } } } --------------020109050107040102010806-- From debbugs-submit-bounces@debbugs.gnu.org Mon May 14 08:44:20 2012 Received: (at 11467) by debbugs.gnu.org; 14 May 2012 12:44:20 +0000 Received: from localhost ([127.0.0.1]:52321 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STudP-000601-St for submit@debbugs.gnu.org; Mon, 14 May 2012 08:44:20 -0400 Received: from mx.meyering.net ([88.168.87.75]:46132) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STudN-0005zo-DZ for 11467@debbugs.gnu.org; Mon, 14 May 2012 08:44:18 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 5359F602D4; Mon, 14 May 2012 14:43:53 +0200 (CEST) From: Jim Meyering To: Rich Burridge Subject: Re: bug#11467: Parfait problems with GNU coreutils In-Reply-To: <4FB0F9A5.7020306@oracle.com> (Rich Burridge's message of "Mon, 14 May 2012 05:25:09 -0700") References: <4FB0F9A5.7020306@oracle.com> Date: Mon, 14 May 2012 14:43:53 +0200 Message-ID: <877gwfgcfq.fsf@rho.meyering.net> Lines: 40 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) Thanks for auditing coreutils! A bug in sort would have been a surprise, and more of an issue, so I've looked at it first. Rich Burridge wrote: ... > Error: Null pointer dereference (CWE 476) > Read from null pointer 's' > at line 3389 of components/coreutils/coreutils-8.5/src/sort.c in function 'main'. > Function 'parse_field_count' may return constant 'NULL' at line 3130, called at line 3387. That is not true when the third argument to parse_field_count is non-NULL, as is the case in sort.c from coreutils-8.5 (and in the latest from git). In that case, parse_field_count exits upon failure and cannot return NULL. Here's the comment/header for that function, and the code that might return NULL: /* Parse the leading integer in STRING and store the resulting value (which must fit into size_t) into *VAL. Return the address of the suffix after the integer. If the value is too large, silently substitute SIZE_MAX. If MSGID is NULL, return NULL after failure; otherwise, report MSGID and exit on failure. */ static char const * parse_field_count (char const *string, size_t *val, char const *msgid) { ... case LONGINT_INVALID: if (msgid) error (SORT_FAILURE, 0, _("%s: invalid count at start of %s"), _(msgid), quote (string)); return NULL; } sort.c checks "s" after the preceding use of parse_field_count, because that invocation passes NULL as the third argument, and hence *can* return NULL. From debbugs-submit-bounces@debbugs.gnu.org Mon May 14 09:58:36 2012 Received: (at 11467) by debbugs.gnu.org; 14 May 2012 13:58:36 +0000 Received: from localhost ([127.0.0.1]:52874 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STvnH-0007s2-Jo for submit@debbugs.gnu.org; Mon, 14 May 2012 09:58:36 -0400 Received: from rcsinet15.oracle.com ([148.87.113.117]:23846) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STvnE-0007rp-Sc for 11467@debbugs.gnu.org; Mon, 14 May 2012 09:58:33 -0400 Received: from acsinet22.oracle.com (acsinet22.oracle.com [141.146.126.238]) by rcsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q4EDw4xr002473 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 14 May 2012 13:58:05 GMT Received: from acsmt357.oracle.com (acsmt357.oracle.com [141.146.40.157]) by acsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q4EDw3Qm009904 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 May 2012 13:58:03 GMT Received: from abhmt117.oracle.com (abhmt117.oracle.com [141.146.116.69]) by acsmt357.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q4EDw32d023069; Mon, 14 May 2012 08:58:03 -0500 Received: from [10.132.144.96] (/10.132.144.96) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 14 May 2012 06:58:03 -0700 Message-ID: <4FB1105A.90100@oracle.com> Date: Mon, 14 May 2012 07:02:02 -0700 From: Rich Burridge User-Agent: Mozilla/5.0 (X11; SunOS i86pc; rv:10.0.3) Gecko/20120424 Thunderbird/10.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#11467: Parfait problems with GNU coreutils References: <4FB0F9A5.7020306@oracle.com> <877gwfgcfq.fsf@rho.meyering.net> In-Reply-To: <877gwfgcfq.fsf@rho.meyering.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: acsinet22.oracle.com [141.146.126.238] X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 05/14/12 05:43 AM, Jim Meyering wrote: > Thanks for auditing coreutils! > A bug in sort would have been a surprise, and more of an issue, > so I've looked at it first. > > Rich Burridge wrote: > ... >> Error: Null pointer dereference (CWE 476) >> Read from null pointer 's' >> at line 3389 of components/coreutils/coreutils-8.5/src/sort.c in function 'main'. >> Function 'parse_field_count' may return constant 'NULL' at line 3130, called at line 3387. > That is not true when the third argument to parse_field_count is > non-NULL, as is the case in sort.c from coreutils-8.5 (and in the > latest from git). In that case, parse_field_count exits upon failure > and cannot return NULL. Thanks Jim. Incomplete analysis on my part. This is a(nother) case where parfait is not smart enough to recognize that error is a non-returning function. From debbugs-submit-bounces@debbugs.gnu.org Mon May 14 10:03:46 2012 Received: (at 11467) by debbugs.gnu.org; 14 May 2012 14:03:46 +0000 Received: from localhost ([127.0.0.1]:52884 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STvsH-00080e-Kq for submit@debbugs.gnu.org; Mon, 14 May 2012 10:03:46 -0400 Received: from mx.meyering.net ([88.168.87.75]:46349) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STvsE-00080V-Ez for 11467@debbugs.gnu.org; Mon, 14 May 2012 10:03:44 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 03542600FD; Mon, 14 May 2012 16:03:18 +0200 (CEST) From: Jim Meyering To: Rich Burridge Subject: Re: bug#11467: Parfait problems with GNU coreutils In-Reply-To: <4FB0F9A5.7020306@oracle.com> (Rich Burridge's message of "Mon, 14 May 2012 05:25:09 -0700") References: <4FB0F9A5.7020306@oracle.com> Date: Mon, 14 May 2012 16:03:18 +0200 Message-ID: <87zk9ag8rd.fsf@rho.meyering.net> Lines: 112 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) Rich Burridge wrote: ... > I've attached a patch that we are applying to the code to fix these problems. > > Here's the evaluation of why the changes have been made: > > There are three different types of Parfait errors here: > > 1/ In fmt.c, in the get_paragraph() routine, Parfait thinks that there > is a potential problem of a negative index into the 'word' array. > But that situation is impossible. Earlier in the routine, get_line() > is called, and this has to have incremented word_limit. The solution > was to add a Parfait style comment before the offending line of code > to shut parfait up. > > 2/ In sort.c, there is an occurrence in the main() routine where 's' was > being dereferenced, but it could have been NULL. A check was added > to only do the dereferencing if that wasn't the case. It was noticed > that there was already similar code in the same routine, so this seems > a reasonable solution. > > 3/ In stty.c, there were two occurrences where bitsp was being dereferenced > that could have been a NULL pointer. In each case, a check was added > to only do the dereferencing if that wasn't the case. It was noticed > that there was already similar code in the sane_mode() routine, so this > seems a reasonable solution. Thanks again. I've just confirmed that your proposed stty.c change is not required, since bitsp cannot be NULL when it is dereferenced. Are the following proposed changes enough to placate parfait? I prefer to use assert, because that tends to work also for static analysis tools like clang and coverity. >From 94f417db5e093093ff9512869880e39975822be8 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 14 May 2012 15:44:41 +0200 Subject: [PATCH] maint: add assertions to placate static analysis tools A static analysis tool (http://labs.oracle.com/projects/parfait/) produced some false positive diagnostics. Add assertions to help it understand that the code is correct. * src/stty.c: Include . (display_changed): Add an assertion to placate parfait. (display_all): Likewise. * src/sort.c: Include . (main): Add an assertion to placate parfait. --- src/sort.c | 5 +++++ src/stty.c | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/sort.c b/src/sort.c index 493e7f1..2593a2a 100644 --- a/src/sort.c +++ b/src/sort.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "system.h" #include "argmatch.h" #include "error.h" @@ -4243,6 +4244,10 @@ main (int argc, char **argv) char const *optarg1 = argv[optind++]; s = parse_field_count (optarg1 + 1, &key->eword, N_("invalid number after '-'")); + /* When called with a non-NULL message ID, + parse_field_count cannot return NULL. Tell static + analysis tools that dereferencing S is safe. */ + assert (s); if (*s == '.') s = parse_field_count (s + 1, &key->echar, N_("invalid number after '.'")); diff --git a/src/stty.c b/src/stty.c index eb07f85..a3fc3dd 100644 --- a/src/stty.c +++ b/src/stty.c @@ -52,6 +52,7 @@ #endif #include #include +#include #include "system.h" #include "error.h" @@ -1538,6 +1539,12 @@ display_changed (struct termios *mode) bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; + + /* bitsp would be NULL only for "combination" modes, yet those + are filtered out above via the OMIT flag. Tell static analysis + tools that it's ok to dereference bitsp here. */ + assert (bitsp); + if ((*bitsp & mask) == mode_info[i].bits) { if (mode_info[i].flags & SANE_UNSET) @@ -1615,6 +1622,7 @@ display_all (struct termios *mode, char const *device_name) bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; + assert (bitsp); /* See the identical assertion and comment above. */ if ((*bitsp & mask) == mode_info[i].bits) wrapf ("%s", mode_info[i].name); else if (mode_info[i].flags & REV) -- 1.7.10.2.484.gcd07cc5 From debbugs-submit-bounces@debbugs.gnu.org Mon May 14 11:37:11 2012 Received: (at 11467) by debbugs.gnu.org; 14 May 2012 15:37:11 +0000 Received: from localhost ([127.0.0.1]:53002 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STxKg-0001pW-Im for submit@debbugs.gnu.org; Mon, 14 May 2012 11:37:11 -0400 Received: from rcsinet15.oracle.com ([148.87.113.117]:33952) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1STxKL-0001of-ID for 11467@debbugs.gnu.org; Mon, 14 May 2012 11:37:09 -0400 Received: from acsinet21.oracle.com (acsinet21.oracle.com [141.146.126.237]) by rcsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q4EFaKSA003767 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 14 May 2012 15:36:20 GMT Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q4EFaJLe022767 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 May 2012 15:36:19 GMT Received: from abhmt111.oracle.com (abhmt111.oracle.com [141.146.116.63]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q4EFaG68014112; Mon, 14 May 2012 10:36:16 -0500 Received: from [10.132.144.96] (/10.132.144.96) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 14 May 2012 08:36:16 -0700 Message-ID: <4FB1276A.8020804@oracle.com> Date: Mon, 14 May 2012 08:40:26 -0700 From: Rich Burridge User-Agent: Mozilla/5.0 (X11; SunOS i86pc; rv:10.0.3) Gecko/20120424 Thunderbird/10.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#11467: Parfait problems with GNU coreutils References: <4FB0F9A5.7020306@oracle.com> <87zk9ag8rd.fsf@rho.meyering.net> In-Reply-To: <87zk9ag8rd.fsf@rho.meyering.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: acsinet21.oracle.com [141.146.126.237] X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 05/14/12 07:03 AM, Jim Meyering wrote: > ... > Thanks again. > I've just confirmed that your proposed stty.c change > is not required, since bitsp cannot be NULL when it is > dereferenced. > > Are the following proposed changes enough to placate parfait? > I prefer to use assert, because that tends to work also for > static analysis tools like clang and coverity. Yup. These changes work just fine. Thanks! > > From 94f417db5e093093ff9512869880e39975822be8 Mon Sep 17 00:00:00 2001 > From: Jim Meyering > Date: Mon, 14 May 2012 15:44:41 +0200 > Subject: [PATCH] maint: add assertions to placate static analysis tools > > A static analysis tool (http://labs.oracle.com/projects/parfait/) > produced some false positive diagnostics. Add assertions to help > it understand that the code is correct. > * src/stty.c: Include. > (display_changed): Add an assertion to placate parfait. > (display_all): Likewise. > * src/sort.c: Include. > (main): Add an assertion to placate parfait. > --- > src/sort.c | 5 +++++ > src/stty.c | 8 ++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/src/sort.c b/src/sort.c > index 493e7f1..2593a2a 100644 > --- a/src/sort.c > +++ b/src/sort.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > #include "system.h" > #include "argmatch.h" > #include "error.h" > @@ -4243,6 +4244,10 @@ main (int argc, char **argv) > char const *optarg1 = argv[optind++]; > s = parse_field_count (optarg1 + 1,&key->eword, > N_("invalid number after '-'")); > + /* When called with a non-NULL message ID, > + parse_field_count cannot return NULL. Tell static > + analysis tools that dereferencing S is safe. */ > + assert (s); > if (*s == '.') > s = parse_field_count (s + 1,&key->echar, > N_("invalid number after '.'")); > diff --git a/src/stty.c b/src/stty.c > index eb07f85..a3fc3dd 100644 > --- a/src/stty.c > +++ b/src/stty.c > @@ -52,6 +52,7 @@ > #endif > #include > #include > +#include > > #include "system.h" > #include "error.h" > @@ -1538,6 +1539,12 @@ display_changed (struct termios *mode) > > bitsp = mode_type_flag (mode_info[i].type, mode); > mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; > + > + /* bitsp would be NULL only for "combination" modes, yet those > + are filtered out above via the OMIT flag. Tell static analysis > + tools that it's ok to dereference bitsp here. */ > + assert (bitsp); > + > if ((*bitsp& mask) == mode_info[i].bits) > { > if (mode_info[i].flags& SANE_UNSET) > @@ -1615,6 +1622,7 @@ display_all (struct termios *mode, char const *device_name) > > bitsp = mode_type_flag (mode_info[i].type, mode); > mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; > + assert (bitsp); /* See the identical assertion and comment above. */ > if ((*bitsp& mask) == mode_info[i].bits) > wrapf ("%s", mode_info[i].name); > else if (mode_info[i].flags& REV) > -- > 1.7.10.2.484.gcd07cc5 From debbugs-submit-bounces@debbugs.gnu.org Tue May 15 16:33:52 2012 Received: (at 11467) by debbugs.gnu.org; 15 May 2012 20:33:52 +0000 Received: from localhost ([127.0.0.1]:56687 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SUORL-0005in-Vq for submit@debbugs.gnu.org; Tue, 15 May 2012 16:33:52 -0400 Received: from mx.meyering.net ([88.168.87.75]:51353) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SUOR1-0005iE-BS for 11467@debbugs.gnu.org; Tue, 15 May 2012 16:33:51 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 371AC6012F; Tue, 15 May 2012 22:33:23 +0200 (CEST) From: Jim Meyering To: Rich Burridge Subject: Re: bug#11467: Parfait problems with GNU coreutils In-Reply-To: <87zk9ag8rd.fsf@rho.meyering.net> (Jim Meyering's message of "Mon, 14 May 2012 16:03:18 +0200") References: <4FB0F9A5.7020306@oracle.com> <87zk9ag8rd.fsf@rho.meyering.net> Date: Tue, 15 May 2012 22:33:23 +0200 Message-ID: <87bolpyyjw.fsf@rho.meyering.net> Lines: 184 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) Jim Meyering wrote: > Rich Burridge wrote: > ... >> I've attached a patch that we are applying to the code to fix these problems. >> >> Here's the evaluation of why the changes have been made: >> >> There are three different types of Parfait errors here: >> >> 1/ In fmt.c, in the get_paragraph() routine, Parfait thinks that there >> is a potential problem of a negative index into the 'word' array. >> But that situation is impossible. Earlier in the routine, get_line() >> is called, and this has to have incremented word_limit. The solution >> was to add a Parfait style comment before the offending line of code >> to shut parfait up. >> >> 2/ In sort.c, there is an occurrence in the main() routine where 's' was >> being dereferenced, but it could have been NULL. A check was added >> to only do the dereferencing if that wasn't the case. It was noticed >> that there was already similar code in the same routine, so this seems >> a reasonable solution. >> >> 3/ In stty.c, there were two occurrences where bitsp was being dereferenced >> that could have been a NULL pointer. In each case, a check was added >> to only do the dereferencing if that wasn't the case. It was noticed >> that there was already similar code in the sane_mode() routine, so this >> seems a reasonable solution. > > Thanks again. > I've just confirmed that your proposed stty.c change > is not required, since bitsp cannot be NULL when it is > dereferenced. > > Are the following proposed changes enough to placate parfait? > I prefer to use assert, because that tends to work also for > static analysis tools like clang and coverity. > > Subject: [PATCH] maint: add assertions to placate static analysis tools > > A static analysis tool (http://labs.oracle.com/projects/parfait/) > produced some false positive diagnostics. Add assertions to help > it understand that the code is correct. > * src/stty.c: Include . > (display_changed): Add an assertion to placate parfait. > (display_all): Likewise. > * src/sort.c: Include . > (main): Add an assertion to placate parfait. Hi Rich, If you can confirm that the following incremental patch is enough to inform parfait that there's no real problem in this part of fmt.c, I'll push the full patch below. diff --git a/src/fmt.c b/src/fmt.c index 308b645..e0c04cb 100644 --- a/src/fmt.c +++ b/src/fmt.c @@ -20,6 +20,7 @@ #include #include #include +#include /* Redefine. Otherwise, systems (Unicos for one) with headers that define it to be a type get syntax errors for the variable declaration below. */ @@ -610,6 +611,11 @@ get_paragraph (FILE *f) while (same_para (c) && in_column == other_indent) c = get_line (f, c); } + + /* Tell static analysis tools that word_limit[-1] is ok. + word_limit is guaranteed to have been incremented by get_line. */ + assert (word < word_limit); + (word_limit - 1)->period = (word_limit - 1)->final = true; next_char = c; return true; >From 5917d6365e03eee7c28c4add3acf79e9f473d703 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 14 May 2012 15:44:41 +0200 Subject: [PATCH] maint: add assertions to placate static analysis tools A static analysis tool (http://labs.oracle.com/projects/parfait/) produced some false positive diagnostics. Add assertions to help it understand that the code is correct. * src/stty.c: Include . (display_changed): Add an assertion to placate parfait. (display_all): Likewise. * src/sort.c: Include . (main): Add an assertion to placate parfait. * src/fmt.c: Include . (get_paragraph): Add an assertion to placate parfait. --- src/fmt.c | 6 ++++++ src/sort.c | 5 +++++ src/stty.c | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/src/fmt.c b/src/fmt.c index 308b645..3da198e 100644 --- a/src/fmt.c +++ b/src/fmt.c @@ -20,6 +20,7 @@ #include #include #include +#include /* Redefine. Otherwise, systems (Unicos for one) with headers that define it to be a type get syntax errors for the variable declaration below. */ @@ -610,6 +611,11 @@ get_paragraph (FILE *f) while (same_para (c) && in_column == other_indent) c = get_line (f, c); } + + /* Tell static analysis tools that using word_limit[-1] is ok. + word_limit is guaranteed to have been incremented by get_line. */ + assert (word < word_limit); + (word_limit - 1)->period = (word_limit - 1)->final = true; next_char = c; return true; diff --git a/src/sort.c b/src/sort.c index 493e7f1..2593a2a 100644 --- a/src/sort.c +++ b/src/sort.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "system.h" #include "argmatch.h" #include "error.h" @@ -4243,6 +4244,10 @@ main (int argc, char **argv) char const *optarg1 = argv[optind++]; s = parse_field_count (optarg1 + 1, &key->eword, N_("invalid number after '-'")); + /* When called with a non-NULL message ID, + parse_field_count cannot return NULL. Tell static + analysis tools that dereferencing S is safe. */ + assert (s); if (*s == '.') s = parse_field_count (s + 1, &key->echar, N_("invalid number after '.'")); diff --git a/src/stty.c b/src/stty.c index eb07f85..a3fc3dd 100644 --- a/src/stty.c +++ b/src/stty.c @@ -52,6 +52,7 @@ #endif #include #include +#include #include "system.h" #include "error.h" @@ -1538,6 +1539,12 @@ display_changed (struct termios *mode) bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; + + /* bitsp would be NULL only for "combination" modes, yet those + are filtered out above via the OMIT flag. Tell static analysis + tools that it's ok to dereference bitsp here. */ + assert (bitsp); + if ((*bitsp & mask) == mode_info[i].bits) { if (mode_info[i].flags & SANE_UNSET) @@ -1615,6 +1622,7 @@ display_all (struct termios *mode, char const *device_name) bitsp = mode_type_flag (mode_info[i].type, mode); mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits; + assert (bitsp); /* See the identical assertion and comment above. */ if ((*bitsp & mask) == mode_info[i].bits) wrapf ("%s", mode_info[i].name); else if (mode_info[i].flags & REV) -- 1.7.10.2.484.gcd07cc5 From debbugs-submit-bounces@debbugs.gnu.org Tue May 15 17:05:37 2012 Received: (at 11467) by debbugs.gnu.org; 15 May 2012 21:05:37 +0000 Received: from localhost ([127.0.0.1]:56710 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SUOw4-0006V9-Ol for submit@debbugs.gnu.org; Tue, 15 May 2012 17:05:37 -0400 Received: from acsinet15.oracle.com ([141.146.126.227]:46763) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SUOvk-0006UZ-50 for 11467@debbugs.gnu.org; Tue, 15 May 2012 17:05:34 -0400 Received: from ucsinet22.oracle.com (ucsinet22.oracle.com [156.151.31.94]) by acsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q4FL527I009606 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 15 May 2012 21:05:03 GMT Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by ucsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q4FL50Qa003202 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 15 May 2012 21:05:01 GMT Received: from abhmt116.oracle.com (abhmt116.oracle.com [141.146.116.68]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q4FL4xiV001185; Tue, 15 May 2012 16:05:00 -0500 Received: from [10.0.1.42] (/24.4.33.245) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 15 May 2012 14:04:59 -0700 Message-ID: <4FB2C4F9.2030703@oracle.com> Date: Tue, 15 May 2012 14:04:57 -0700 From: Rich Burridge User-Agent: Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#11467: Parfait problems with GNU coreutils References: <4FB0F9A5.7020306@oracle.com> <87zk9ag8rd.fsf@rho.meyering.net> <87bolpyyjw.fsf@rho.meyering.net> In-Reply-To: <87bolpyyjw.fsf@rho.meyering.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: ucsinet22.oracle.com [156.151.31.94] X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 05/15/2012 01:33 PM, Jim Meyering wrote: > If you can confirm that the following incremental patch is enough to > inform parfait that there's no real problem in this part of fmt.c, > I'll push the full patch below. Works great. Thanks for doing this. From debbugs-submit-bounces@debbugs.gnu.org Sun Oct 07 05:55:20 2012 Received: (at 11467) by debbugs.gnu.org; 7 Oct 2012 09:55:20 +0000 Received: from localhost ([127.0.0.1]:59071 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TKnZs-00035K-KG for submit@debbugs.gnu.org; Sun, 07 Oct 2012 05:55:18 -0400 Received: from mx.meyering.net ([88.168.87.75]:40132) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TKnZq-000358-0H; Sun, 07 Oct 2012 05:55:14 -0400 Received: from rho.meyering.net (rho.meyering.net [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 2139B600DB; Sun, 7 Oct 2012 11:54:52 +0200 (CEST) From: Jim Meyering To: Rich Burridge Subject: Re: bug#11467: Parfait problems with GNU coreutils In-Reply-To: <4FB1105A.90100@oracle.com> (Rich Burridge's message of "Mon, 14 May 2012 07:02:02 -0700") References: <4FB0F9A5.7020306@oracle.com> <877gwfgcfq.fsf@rho.meyering.net> <4FB1105A.90100@oracle.com> Date: Sun, 07 Oct 2012 11:54:52 +0200 Message-ID: <87k3v2wqs3.fsf@rho.meyering.net> Lines: 25 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -1.3 (-) X-Debbugs-Envelope-To: 11467 Cc: 11467@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.3 (-) tags 11467 + notabug close 11467 thanks Rich Burridge wrote: > On 05/14/12 05:43 AM, Jim Meyering wrote: >> Thanks for auditing coreutils! >> A bug in sort would have been a surprise, and more of an issue, >> so I've looked at it first. >> >> Rich Burridge wrote: >> ... >>> Error: Null pointer dereference (CWE 476) >>> Read from null pointer 's' >>> at line 3389 of components/coreutils/coreutils-8.5/src/sort.c in function 'main'. >>> Function 'parse_field_count' may return constant 'NULL' at line 3130, called at line 3387. >> That is not true when the third argument to parse_field_count is >> non-NULL, as is the case in sort.c from coreutils-8.5 (and in the >> latest from git). In that case, parse_field_count exits upon failure >> and cannot return NULL. > > Thanks Jim. Incomplete analysis on my part. This is a(nother) case where > parfait is not smart enough to recognize that error is a non-returning function. This is resolved, so closing. From unknown Tue Jun 24 15:45:02 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sun, 04 Nov 2012 12:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator