From owner-freebsd-bugs@FreeBSD.ORG Thu Nov 17 00:50:15 2005 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8310816A41F for ; Thu, 17 Nov 2005 00:50:15 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id EF0CE43D49 for ; Thu, 17 Nov 2005 00:50:14 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id jAH0oEwf078496 for ; Thu, 17 Nov 2005 00:50:14 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id jAH0oEs1078495; Thu, 17 Nov 2005 00:50:14 GMT (envelope-from gnats) Resent-Date: Thu, 17 Nov 2005 00:50:14 GMT Resent-Message-Id: <200511170050.jAH0oEs1078495@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Antony Mawer Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1EC7416A41F for ; Thu, 17 Nov 2005 00:44:38 +0000 (GMT) (envelope-from ajmawer@mawer.org) Received: from mail24.syd.optusnet.com.au (mail24.syd.optusnet.com.au [211.29.133.165]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4485F43D45 for ; Thu, 17 Nov 2005 00:44:36 +0000 (GMT) (envelope-from ajmawer@mawer.org) Received: from scooby.enchanted.net (c220-237-120-88.thorn1.nsw.optusnet.com.au [220.237.120.88]) by mail24.syd.optusnet.com.au (8.12.11/8.12.11) with ESMTP id jAH0iZkk016928 for ; Thu, 17 Nov 2005 11:44:35 +1100 Received: by scooby.enchanted.net (Postfix, from userid 1001) id EE3597A; Thu, 17 Nov 2005 11:44:34 +1100 (EST) Message-Id: <20051117004434.EE3597A@scooby.enchanted.net> Date: Thu, 17 Nov 2005 11:44:34 +1100 (EST) From: Antony Mawer To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/89181: [patch] lib/libutil properties_read() bug with files > 4096 bytes X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Antony Mawer List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Nov 2005 00:50:15 -0000 >Number: 89181 >Category: kern >Synopsis: [patch] lib/libutil properties_read() bug with files > 4096 bytes >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Nov 17 00:50:14 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Antony Mawer >Release: FreeBSD 6.0-BETA2 i386 >Organization: GP Technology Solutions >Environment: System: FreeBSD scooby.enchanted.net 6.0-BETA2 FreeBSD 6.0-BETA2 #1: Mon Aug 8 09:56:30 EST 2005 root@scooby.enchanted.net:/usr/obj/usr/src/sys/SCOOBY i386 >Description: While developing a custom sysinstall-based 6.0 installation CD, I hit upon a "Invalid realloc size of 0!" error during testing. This problem occurs every time at exactly the same place during the installation, and happens when installing a (relatively) large local distribution (~200mb). Looking at the code, I suspected it was a problem with the property functions in libutil, and wrote a test program that parsed the local.inf file. The source for this test program can be found at the following URL, allong with the local.inf that demonstrates the problem: http://people.gptech.com.au/~ajmawer/freebsd/proptest.c http://people.gptech.com.au/~ajmawer/freebsd/local.inf The test program indicated that in the middle of the property values read from the file, one was returning NULL, while subsequent properties were able to be read without a problem: Processing chunk 137 of 163 Looking for cksum.fh Read property '(null)' Tracing through properties_read() in src/lib/libutil/property.c showed that the first 4096 bytes were being into a buffer and processed, then the function would read another 4096 and begin in a LOOK state again, regardless of where it was up to prior to the buffer running out. As a result, the property that was half-way through being read was discarded and properties_read() started work on the next one. >How-To-Repeat: 1. Download the proptest.c and local.inf files and place them in the same directory. 2. Compile the proptest.c file: gcc -o proptest -lutil proptest.c 3. Run the command: ./proptest | less 4. Observe that Read property for 'cksum.fh' returns NULL >Fix: The problem is that when properties_read() reaches the end of its, it sets the state = FILL, discarding whatever the previous state was, re-fills the buffer, and then launches straight into a LOOK state by allowing the switch statement to fall through to the next case statement. So if, as in this instance, the buffer runs out while it is in the middle of a state = VALUE operation, this state is lost and a fresh search for a name=value pair begins. The attached patch teaches properties_read() to remember its current state prior to setting state = FILL, and restore it upon refilling the buffer. In order to do this, it was necessary to remove the fallthrough to the LOOK case, and instead re-start the while loop and re-evaluate the state. After rebuilding with the attached patch, re-running the proptest.c shows that the cksum.fh is now read correctly. --- property.c.diff begins here --- index: lib/libutil/property.c =================================================================== RCS file: /home/ncvs/src/lib/libutil/property.c,v retrieving revision 1.13 diff -u -r1.13 property.c --- lib/libutil/property.c 14 Jun 2003 18:42:37 -0000 1.13 +++ lib/libutil/property.c 17 Nov 2005 00:03:41 -0000 @@ -75,17 +75,18 @@ char hold_v[PROPERTY_MAX_VALUE + 1]; char buf[BUFSIZ * 4]; int bp, n, v, max; - enum { LOOK, COMMENT, NAME, VALUE, MVALUE, COMMIT, FILL, STOP } state; + enum { LOOK, COMMENT, NAME, VALUE, MVALUE, COMMIT, FILL, STOP } state, last_state; int ch = 0, blevel = 0; n = v = bp = max = 0; head = ptr = NULL; - state = LOOK; + state = last_state = LOOK; while (state != STOP) { if (state != COMMIT) { - if (bp == max) + if (bp == max) { + last_state = state; state = FILL; - else + } else ch = buf[bp++]; } switch(state) { @@ -96,13 +97,17 @@ } if (max == 0) { state = STOP; - break; } else { - state = LOOK; + /* Restore the state as to before the fill (which will be + * initialised to LOOK for the first FILL). This ensures that + * if we were part-way through, eg. a VALUE state, when the + * buffer ran out, that the previous operation will be allowed + * to complete. */ + state = last_state; ch = buf[0]; - bp = 1; + bp = 0; } - /* FALLTHROUGH deliberately since we already have a character and state == LOOK */ + continue; case LOOK: if (isspace((unsigned char)ch)) --- property.c.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted: