stdio self-synchronized destruction: does it need fixing?

stdio self-synchronized destruction: does it need fixing?

[Rich Felker]

Hi all,
Question about an issue that seems purely theoretical:
self-synchronized destruction of stdio FILEs. Does anything need to be
fixed?

The basic self-synchronized destruction scenario that affects all
synchronization primitives is: Thread A is waiting to acquire the
resource with the knowledge that there is presently at most one other
thread using it. Thread B releases the lock, then checks to see if
there are any waiters to wake up. But between these two steps, it's
possible that thread A already woke up and destroyed the object, in
which case, reading the waiters field is an invalid read.

glibc has a number of such bugs, but I fixed most of them in musl a
long time ago. (Note that this is a race condition that takes
somewhere on the order of years to hit, unless you artificially poke
at the timing; that's why it doesn't get much attention.) Anyway, I
recently realized that musl still has the issue for stdio FILE
streams. If thread B is holding a lock with flockfile and thread A
calls fclose on the locked stream, it's possible that when thread B
unlocks the stream, thread A gets to free it before thread B can check
to see if there are waiters that need to be woken.

"Fixing" the issue would not be too hard, but since it's nontrivial,
I'd like to consider whether the fix is actually necessary. Unlike
mutexes or semaphores, FILEs do not exist in application-controlled
memory. The allocation of FILE structues is always performed by libc,
and always happens via malloc with a small-size allocation, which
means the memory is managed as part of the heap and never unmapped
once it's mapped. Thus, as far as I can tell, the worst that can
happen is a read-only access to memory no longer owned by the FILE,
but that's still valid, and in this case, it doesn't matter whether a
wakeup futex event is generated, since there can be no waiters;
spurious wakeup events are harmless here.

Any thoughts by others on the matter?

Rich

Re: stdio self-synchronized destruction: does it need fixing?

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2012-12-10 13:05:09 -0500]:
> memory. The allocation of FILE structues is always performed by libc,
> and always happens via malloc with a small-size allocation, which
> means the memory is managed as part of the heap and never unmapped
> once it's mapped. Thus, as far as I can tell, the worst that can
> happen is a read-only access to memory no longer owned by the FILE,

at least write a comment there that the invalid read is known

(btw at some point someone may rewrite malloc so small allocations
can go to mmapped areas as well which may be reclaimed..)

Re: stdio self-synchronized destruction: does it need fixing?

[Rich Felker]

On Mon, Dec 10, 2012 at 11:57:53PM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@aerifal.cx> [2012-12-10 13:05:09 -0500]:
> > memory. The allocation of FILE structues is always performed by libc,
> > and always happens via malloc with a small-size allocation, which
> > means the memory is managed as part of the heap and never unmapped
> > once it's mapped. Thus, as far as I can tell, the worst that can
> > happen is a read-only access to memory no longer owned by the FILE,
> 
> at least write a comment there that the invalid read is known

This sounds like a perfect solution for now.

> (btw at some point someone may rewrite malloc so small allocations
> can go to mmapped areas as well which may be reclaimed..)

I would like to add the ability to obtain more heap memory from mmap,
but I think unmapping unused parts has more cons than pros. The only
benefit is that it reclaims the virtual address space so that the
kernel can use it for honoring future mmap requests. The disadvantage
is that it fragments the heap; once a "hole" has been created in the
heap, it's impossible to put it back and merge the adjacent free
regions into one large free region in O(1) time.

Of course if you're talking about a _whole_ supplemental mmap zone
becoming free, then it would be safe to unmap it, and this may be a
worthwhile case to handle. In particular, one way of doing
supplemental heap via mmap would be to make each map roughly the same
size as the threshold for releasing it back to the kernel via
mprotect/mmap zeroing. The beginning and end of the region could have
specially-flagged chunks, and then, after merging on free(), if both
the previous and next neighbor are flagged as the mmap region
endpoints, the whole region could be unmapped rather than adding it
back to the free list.

The alternative would be to do supplemental mmap regions in much
larger increments, maybe on the order of 16MB, initially with
PROT_NONE, and grow the PROT_READ|PROT_WRITE portion like brk() does
for the main heap. Then the same code that releases parts of the main
brk heap via zeroing could also work well here. I think this
alternative approach would result in less fragmentation for malloc,
but more chance of filling up virtual address space and preventing
mmap from working.

Rich