kern/89181: [patch] lib/libutil properties_read() bug with files >
4096 bytes
Antony Mawer
gnats at mawer.org
Thu Nov 17 00:50:15 GMT 2005
>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 at 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:
More information about the freebsd-bugs
mailing list