This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: cat /proc/registry/HKEY_PERFOMANCE_DATA/@ hangs


> -----Original Message-----
> From: cygwin-owner On Behalf Of Igor Pechtchanski
> Sent: 14 July 2004 04:22

> Ok, the theory washed out.  The code above is actually simply 
> buggy.  When
> RegQueryValueEx is called (2 lines below the arrow), the 
> "size" parameter
> is uninitialized, so, in effect, it keeps thinking that the buffer has
> some random size and reallocating (which, of course, doesn't 
> change the
> size, hence the infinite loop).

  I concur; that is bad code.  The variable unambiguously needs
initialising, and since RegQueryValueEx damages it, it needs to be re-set
each time round the loop.  

  I disagree with your analysis, though.  After the first time round the
loop, size is no longer uninitialised.  After all, the call to
RegQueryValueEx sets size to the amount of space needed.  Second time round
the loop, we set bufalloc to 2000, realloc that much space, then call
RegQueryValueEx, passing it the pointer to this 2000 byte buffer, and the
size variable is still whatever RegQueryValueEx set it to last time, i.e.
the full size of the value's data.  So when we call RegQueryValueEx the
second time, it thinks the buffer is loads bigger than it actually is - in
fact, it thinks the buffer is exactly big enough for the value's data - and
it copies waaaay more data into the buffer than it has room for.  Bang!
Heap corruption!

> However, the fix is not as simple as inserting a "size = 
> bufalloc;" just
> before the RegQueryValueEx.  When I do that, I get a SIGSEGV 
> in the guts
> of iasperf.dll, which I have yet to track down.  This happens on the
> second iteration, FWIW, with buffer increment of 1000.  

  That's funny.  It WFMHRN.  Are you completely sure that you put that line
in the right place?  Are you sure you remembered to rebuild the dll after
editing the source?  Are you sure you tested the new version of the dll
rather than some old one?  I know some of these things sound daft, but since
the fix is a) obviously correct, and b) tested and working, I suspect that
you made some little error in the procedure that invalidated your test.  You
wouldn't believe how often I ALT+TAB to another window and type "make all"
only to realise I haven't saved all the changed files that I have in my
editor window, only some of them!  In particular, the fact that you see a
SEGV the second time round - which is what my analysis above demonstrates
should happen if the size variable is NOT reinited each time round the loop
- makes me think you didn't actually test the right code.  You better
double-check.

  Just to show that the bug really is fixed by that one-line patch, here's a
dump of it WFMHRNing:

dk@mace ~/reg-test> ls -la
total 78
drwxr-xr-x+   2 dk       Domain U        0 Jul 14 17:44 .
drwxrwxrwx+   6 dk       Domain U        0 Jul 14 17:43 ..
-rw-r--r--    1 dk       Domain U    79264 Jul 14 17:44 test1.dat
dk@mace ~/reg-test> cat /proc/registry/HKEY_PERFOMANCE_DATA/@ >test2.dat
dk@mace ~/reg-test> cat /proc/registry/HKEY_PERFOMANCE_DATA/@ >test3.dat
dk@mace ~/reg-test> cat /proc/registry/HKEY_PERFOMANCE_DATA/@ >test4.dat
dk@mace ~/reg-test> ls -la
total 312
drwxr-xr-x+   2 dk       Domain U        0 Jul 14 17:44 .
drwxrwxrwx+   6 dk       Domain U        0 Jul 14 17:43 ..
-rw-r--r--    1 dk       Domain U    79264 Jul 14 17:44 test1.dat
-rw-r--r--    1 dk       Domain U    79264 Jul 14 17:44 test2.dat
-rw-r--r--    1 dk       Domain U    79264 Jul 14 17:44 test3.dat
-rw-r--r--    1 dk       Domain U    79264 Jul 14 17:44 test4.dat
dk@mace ~/reg-test>

  Here's the patch, against current CVS.  I'll let Igor supply the ChangeLog
entry, since it was his fix.

Index: winsup/cygwin/fhandler_registry.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_registry.cc,v
retrieving revision 1.24
diff -p -u -r1.24 fhandler_registry.cc
--- winsup/cygwin/fhandler_registry.cc  9 Feb 2004 04:04:23 -0000       1.24
+++ winsup/cygwin/fhandler_registry.cc  14 Jul 2004 16:47:53 -0000
@@ -575,6 +575,7 @@ fhandler_registry::fill_filebuf ()
        {
          bufalloc += 1000;
          filebuf = (char *) realloc (filebuf, bufalloc);
+          size = bufalloc;
          error = RegQueryValueEx (handle, value_name, NULL, &type,
                                   (BYTE *) filebuf, &size);
          if (error != ERROR_SUCCESS && error != ERROR_MORE_DATA)



    cheers, 
      DaveK
-- 
Can't think of a witty .sigline today....


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]