]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commit
Remove NOT_TESTED() from EINTR handling in lib/file
authorOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:23:33 +0000 (11:23 -0700)
committerOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:23:33 +0000 (11:23 -0700)
commitc024d866ac746474487fea646f4787875e2dacdd
treee5fc7d826d52d912ac0ced5f6e97eda41ce2c97d
parentba915c1445124f6369a49788b9be2e8c2886bc10
Remove NOT_TESTED() from EINTR handling in lib/file

When FileIO_Read() & friends receive EINTR, they log NOT_TESTED, and
retry read.  It seems innocent - until you provide your own function
to handle logging.

Now imagine that you create bidirectional communication protocol,
and run it over the pipe.  And you do logging over this very same
pipe.  Application is waiting for message, so it invoked
FileIO_Read(), and is blocked on read().  Now you attach strace
to the process.  That will interrupt pending read() with EINTR
and log NOT_TESTED().  This logging performs FileIO_Write() on
this very same pipe, and then performs FileIO_Read() to retrieve
status of logging request.

If it receives status of logging request, all is good.  Code
returns from logging, and reexecutes read(), waiting for
request from server.

But if logging function receives anything else (f.e. other side
sends request at same time strace was attached), it gets queued
into internal data structures of the app for processing once
code returns to main application loop.

Problem is that FileIO_Read() will reenter read() without
returning to the caller.  And that read() will block forever:
other end already send request that is now pending in the
request list, and so won't send anything until it sees
we processed that pending request.  Hang.

There are two possible fixes:

1. Return EINTR from FileIO_Read(), or

2. Do not do any logging from FileIO_Read()

Approach #2 is much easier, as API semantic does not change,
and currently it is broken.  So there is no need for NOT_TESTED()
anymore...
open-vm-tools/lib/file/fileIOPosix.c