[PATCH] Fix possible deadlock in stdio locking code

[Siddhesh Poyarekar <spoyarek at redhat dot com>]

The macro _IO_unlock_lock decrements the lock count before it tries to
unlock its lock and without actually checking if the count is
positive. This can be risky if the lock was never held in the first
place.

In the current code base, a deadlock may occur as a result of this, in
the following scenario as seen from libio/genops.c, in _IO_un_link
function. Similar argument can be formulated for other functions
inside genops.c that follow similar sequence:

1) thread A pushes the cleanup callback flush_cleanup using
   _IO_cleanup_region_start_noarg()

2) Some other thread calls pthread_cancel on thread A before it gets
   to call _IO_lock_lock(list_all_lock)

3) flush_cleanup() is called while cleaning up cancelled thread A,
   where _IO_lock_unlock(list_all_lock) is called

3) Some thread makes calls that result in calls _IO_lock_lock(list_all_lock)
   and _IO_lock_unlock(list_all_lock)

4) Some thread makes a call that results in _IO_lock_lock(list_all_lock)

Here, (4) will result in a deadlock, since _IO_unlock_lock() in (3)
does not actually release the lock due to the lock count falling to
below zero and hence not being equal to zero.

The trivial patch below fixes this. make check was not of any help
since recent changes seem to have broken some tests. I haven't been
able to arrive at a test case that actually reproduces this; I just
have a core dump (that I unfortunately cannot share) that shows the
lock count as negative, showing that the above scenario is possible.


nptl/ChangeLog:

2011-10-18  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* nptl/sysdeps/pthread/bits/stdio-lock.h: Decrement lock count
	only if it is positive.


diff --git a/nptl/sysdeps/pthread/bits/stdio-lock.h b/nptl/sysdeps/pthread/bits/stdio-lock.h
index b8efdd8..b3f1b75 100644
--- a/nptl/sysdeps/pthread/bits/stdio-lock.h
+++ b/nptl/sysdeps/pthread/bits/stdio-lock.h
@@ -69,7 +69,7 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t;
 
 #define _IO_lock_unlock(_name) \
   do {									      \
-    if (--(_name).cnt == 0)						      \
+    if ((_name).cnt && --(_name).cnt == 0)				      \
       {									      \
         (_name).owner = NULL;						      \
 	lll_unlock ((_name).lock, LLL_PRIVATE);				      \