On 01.06.2012 17:41, Eric Blake wrote:
It is possible to deadlock libvirt by having a domain with XML
longer than PIPE_BUF, and by writing a hook script that closes
stdin early. This is because libvirt was keeping a copy of the
child's stdin read fd open, which means the write fd in the
parent will never see EPIPE (remember, libvirt should always be
run with SIGPIPE ignored, so we should never get a SIGPIPE signal).
Since there is no error, libvirt blocks waiting for a write to
complete, even though the only reader is also libvirt. The
solution is to ensure that only the child can act as a reader
before the parent does any writes; and then dealing with the
fallout of dealing with EPIPE.
Thankfully, this is not a security hole - since the only way to
trigger the deadlock is to install a custom hook script, anyone
that already has privileges to install a hook script already has
privileges to do any number of other equally disruptive things
to libvirt; it would only be a security hole if an unprivileged
user could install a hook script to DoS a privileged user.
* src/util/command.c (virCommandRun): Close parent's copy of child
read fd earlier.
(virCommandProcessIO): Don't let EPIPE be fatal; the child may
be done parsing input.
* tests/commandhelper.c (main): Set up a SIGPIPE situation.
* tests/commandtest.c (test20): Trigger it.
* tests/commanddata/test20.log: New file.
---
I tested this by using just the changes to 'tests/', and was able
to trigger deadlock in 100% of my testing with the buffer lengths
shown here.
src/util/command.c | 22 +++++++++++++---------
tests/commanddata/test20.log | 13 +++++++++++++
tests/commandhelper.c | 6 ++++++
tests/commandtest.c | 42 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 74 insertions(+), 9 deletions(-)
create mode 100644 tests/commanddata/test20.log
So we have now covered this case in tests. Awesome.
ACK
Michal