Bug 14267

Summary: fprintf() function is multithread-unsafe
Product: glibc Reporter: Peng Haitao <penght>
Component: stdioAssignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: aoliva, carlos, davem, drepper.fsp, fweimer, ondra
Priority: P2 Flags: fweimer: security-
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: The Patch can make fprintf() function to multithread-safe

Description Peng Haitao 2012-06-20 05:32:25 UTC
Created attachment 6460 [details]
The Patch can make fprintf() function to multithread-safe

fprintf() uses static variables __printf_function_table and
__printf_va_arg_table indirectly, which are not protected. It is
not safe in multithread circumstance.

When call fprintf() and register_printf_specifier() simultaneously
in multithread circumstance, the following case will cause unsafe.
The case has two threads: A and B.

a. thread B call register_printf_specifier('W', print_widget, print_widget_arginfo)
b. thread A call fprintf (stdout, "|%W|\n", &mywidget), when judge
   __printf_function_table is not NULL, but not output &mywidget
c. thread B call register_printf_specifier('W', NULL, NULL)
d. thread A output &mywidget when __printf_function_table is NULL
e. programme will Segmentation fault
Comment 1 David S. Miller 2012-12-03 22:12:33 UTC
This change should be rearranged such that we only take the lock if the feature in question is used, rather than unconditionally.  Otherwise non-users of the feature take a performance hit, and that's undesirable.
Comment 2 Carlos O'Donell 2013-05-22 14:54:19 UTC
Peng,

I agree with David, these changes need to be considered carefully because fprintf() is a function that everyone uses. Simply adding a lock here is going to impact performance for everyone.

I suggest you follow Alex's posted patches and the discussion threads here:
http://sourceware.org/ml/libc-alpha/2013-03/msg00581.html
http://sourceware.org/ml/libc-alpha/2013-04/msg00023.html
http://sourceware.org/ml/libc-alpha/2013-05/msg00120.html

The first step is going to be to document the exact contract we want to have with our users, including possibly declaring that register_printf_specifier() is MT-unsafe, but that fprintf() *IS* MT-safe with exceptions (don't call register_printf_specifier).

I encourage you to review Alex's work and provide feedback and help submit bugs where appropriate.