Bug 217041 - libioP.h uses mmap() for ALLOC_BUF, which is used in stdio operations a lot
Summary: libioP.h uses mmap() for ALLOC_BUF, which is used in stdio operations a lot
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: glibc
Version: 6
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On: 217064
Blocks: 418441
TreeView+ depends on / blocked
 
Reported: 2006-11-23 12:12 UTC by Arjan van de Ven
Modified: 2008-03-30 06:25 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-03-30 06:25:02 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch to use malloc/free rather than mmap/munmap for stdio buffers (1.31 KB, patch)
2006-11-23 16:42 UTC, Arjan van de Ven
no flags Details | Diff
automated testcase for this behavior (1.49 KB, text/x-csrc)
2006-11-27 13:29 UTC, Arjan van de Ven
no flags Details
updated patch to use malloc/free rather than mmap/munmap; this patch uses calloc based on glibc-alpha feedback (1.63 KB, text/x-patch)
2006-11-27 13:37 UTC, Arjan van de Ven
no flags Details

Description Arjan van de Ven 2006-11-23 12:12:49 UTC
Description of problem:

For some reason, ALLOC_BUF() in libioP.h uses a hardcoded mmap() operation if
the system supports mmap, rather than malloc().. (well it uses that when there
is no mmap, which kinda suggests it's probably ok to call malloc). This results
in a mmap/munmap operation for each stdio file open/read/close cycle; while if
malloc would have been used, the entire syscalls would have been avoided after
the first time due to the hysteresis effect of the brk area.

munmap (but to some degree mmap) are a scalability and performance issue since
they need to take very wide locks and flush the TLB on multiple cpus (in
threaded applications) so in general it's nicer to use malloc.. (and anyway
malloc is designed to go as fast as possible, might as well use that).

Of course I can be missing something big (the obvious question is if ALLOC_BUF
is used in places where malloc() locks are held so that it would
recurse-on-deadlock).. but if there's nothing like that this might be worth
considering...

Comment 1 Arjan van de Ven 2006-11-23 13:24:37 UTC
(a naive replacement doesn't work; investigating why now)

Comment 2 Arjan van de Ven 2006-11-23 15:21:15 UTC
ok I found the original bug, but now I'm hitting a fun snatch...

the tst_wprintf2 test does:

  setvbuf (fp, NULL, _IONBF, 0);

  /* fwprintf to unbuffered stream.   */
  fwprintf (fp, L"hello.\n");

  fclose (fp);

which with a malloc() implementation causes the free() sanity checks to go Bang
in the fclose(fp).
The thing is, I don't know if this is a bug due to my replacing with malloc, or
just an existing hidden bug that went unnoticed due to lack of sanity checks in
the mmap based implementation....


Comment 3 Arjan van de Ven 2006-11-23 15:49:52 UTC
ok this looks like an actual glibc bug; a strace of the testcase on a normal,
unpatches glibc shows:
close(4)                                = 0
munmap(0x604063, 4096)                  = -1 EINVAL (Invalid argument)


so it randomly munmaps a piece of memory and the kernel tells it to bugger off.
Sounds like something really dangerous to do in a threaded environment where
something real might have been mmap'd at this location.... or something might
accidentally be page aligned there, and then some random piece of memory gets
unmapped!


Comment 4 Arjan van de Ven 2006-11-23 16:42:23 UTC
Created attachment 141999 [details]
patch to use malloc/free rather than mmap/munmap for stdio buffers

Once bug #217064 is fixed, this patch switches glibc to just using malloc/free
for better error checking (yay) and for higher performance

Comment 5 Arjan van de Ven 2006-11-27 13:29:24 UTC
Created attachment 142166 [details]
automated testcase for this behavior

Attached is a test program that returns EXIT_FAILURE on the inefficient
behavior and EXIT_SUCCESS when this inefficient behavior does not happen

Comment 6 Arjan van de Ven 2006-11-27 13:37:30 UTC
Created attachment 142169 [details]
updated patch to use malloc/free rather than mmap/munmap; this patch uses calloc based on glibc-alpha feedback

Comment 7 Ulrich Drepper 2008-03-30 06:25:02 UTC
The code deliberately uses mmap.  This makes stream usable even in situations
when malloc is corrupted.


Note You need to log in before you can comment on or make changes to this bug.