Bug 3317 - Overriding free which clobbers data and calls pthread_getspecific can get bad value
Summary: Overriding free which clobbers data and calls pthread_getspecific can get bad...
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.3.6
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-07 01:38 UTC by Ian Lance Taylor
Modified: 2016-08-22 13:44 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Test case (606 bytes, text/plain)
2006-10-07 01:39 UTC, Ian Lance Taylor
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Lance Taylor 2006-10-07 01:38:16 UTC
If I override the libc free() function, and change that function to clobber some
bytes of the data, and then call pthread_getspecific from that function,
pthread_getspecific can return bad data.  This is because __nptl_deallocate_tsd
frees data before clearing the pointer to that data.  This happens around line 185:

	      /* The first block is allocated as part of the thread
		 descriptor.  */
	      free (level2);
	      THREAD_SETMEM_NC (self, specific, cnt, NULL);

The order of those lines should be reversed.

I first noticed this bug in linuxthreads, and then discovered that NPTL had the
same bug.  I will file a separate bug against linuxthreads.

I will attach a test case which shows the problem on i686-pc-linux-gnu using
Fedora Core 4 with glibc-2.3.6-3.
Comment 1 Ian Lance Taylor 2006-10-07 01:39:22 UTC
Created attachment 1356 [details]
Test case

This test case crashes for me on i686-pc-linux-gnu running Fedora Core 4 with
glibc-2.3.6-3.	In the __libc_free routine, pthread_getspecific returns
garbage.
Comment 2 Ulrich Drepper 2006-10-07 20:46:30 UTC
That's plainly invalid code.  You cannot use the TSD anymore when it gets destroyed.
Comment 3 Ian Lance Taylor 2006-10-12 05:58:55 UTC
That was, of course, just an example which tests for whether the problem exists.

The issue is overriding malloc(), free() and friends, where they use pthread
specific keys, and where free clobbers the block data for debugging purposes. 
Since the pthread code itself calls free, it is impossible for free to know
whether or not the pthread specific key has been destroyed.  The only way is for
free to check whether pthread_get_specific returns NULL, but that doesn't work
if it clobbers the data for debugging purposes.

I can't say that I'm surprised that you plan to ignore this bug report. 
However, I hope the report will give a hint to distros to swap the order of
those two lines.
Comment 4 Ulrich Drepper 2006-10-12 21:18:23 UTC
THere will be no change.  You are introducing wrong behavior.  If this wrong
code should be worked around with a patch like this then somebody would complain
that TSD data is left behind and demand this is handled.  It's a rats nest.  If
your program is invalid it must crash ASAP.
Comment 5 Ian Lance Taylor 2006-10-13 05:26:46 UTC
Thank you for your thoughtful and detailed analysis showing why this useful
approach to memory allocation and debugging is invalid.  Thank for your decision
to not swap two lines of code, as it is clearly a slippery slope to destroying
libc.  As always, I stand astounded by your technical decisions and your ability
to communicate.