This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v7] getrandom system call wrapper [BZ #17252]

On 11/17/2016 10:24 AM, Florian Weimer wrote:
> On 11/17/2016 02:56 PM, Zack Weinberg wrote:
>> What I'm asking for is evidence that that is actually a problem for at
>> least one real application.
> I found this:
> <>
> ipsec_resolver_thread reads from /dev/random, which can block for a very
> long time.  The thread for it is spawned in ipsec_process_prefs, which
> cancels it when got_terminate returns true.  The intent seems to be that
> is terminated on graceful process termination: read in
> ipsec_process_prefs returns EINTR after a signal handler has run which
> makes got_terminate (defined in main.c in the parent directory) return
> true.

It seems to me that this code does not *need* to cancel the
ipsec_resolver_thread when it does, as it is about to call exit() [I
think] which will blow away all threads anyway.

>> Also evidence that making getrandom a
>> cancellation point _won't_ break programs that naively assume it can
>> never fail.
> Cancellation does not add additional error return cases.

It does add additional _failure_ cases.  Suppose a program that expects
threads only to be at risk of cancellation at points where they do
network I/O, and does all the necessary dancing to make that reliable.
These threads are _already_ using getrandom() where available, via a
portability wrapper that will call into the C library if possible, or
make a direct syscall otherwise.  Being a wrapper, it's not a
cancellation point, and the surrounding code relies on that.  Now you
upgrade glibc, and suddenly getrandom() _is_ a cancellation point, and
the threads can now be cancelled in places where their data structures
are internally inconsistent -- and it doesn't matter that getrandom()
doesn't block under normal conditions, because the generic
cancel-testing code will fire anyway.

[This is just the general argument that adding new cancellation points
to the C library can render existing code buggy without notice.]


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]