Re: [External] Re: [PATCH v1] security_manager: Ensure top lock is acquired before nested locks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Yes, we have recently encountered this issue, and it is exactly the same as the scenario described in the bug report. We investigated the code and found this potential problem. The issue occurs when collecting information through the 'virsh capabilities' while concurrently creating vm. The higher the frequency and concurrency, the more likely the issue will reproduce. We believe this is a very rare bug; our environment has been running for three years, and we've only encountered this issue once. To successfully reproduce the problem, we constructed a test case by adding a sleep between the lock nested and unlock. But we still need to try to fix this issue, even though the probability is very low.

在 2024/7/3 8:49 下午, Michal Prívozník 写道:
On 6/25/24 12:03, hongmianquan via Devel wrote:
We need to ensure top lock is acquired before nested lock. Otherwise deadlock
issues may arise. We have the stack security driver, which internally manages
other security drivers, we call them "top" and "nested".

We call virSecurityStackPreFork() to lock the top one, and it also locks
and then unlocks the nested drivers prior to fork. Then in qemuSecurityPostFork(),
it unlocks the top one, but not the nested ones. Thus, if one of the nested
drivers ("dac" or "selinux") is still locked, it will cause a deadlock.
We discovered this case: the nested list obtained through the qemuSecurityGetNested()
will be locked for subsequent use, such as in virQEMUDriverCreateCapabilities(),
where the nested list is locked using qemuSecurityGetDOI, but the top one is not locked beforehand.

The problem stack is as follows:

libvirtd thread1          libvirtd thread2          child libvirtd
         |                           |                       |
         |                           |                       |
virsh capabilities      qemuProcessLanuch                   |
         |                           |                       |
         |                       lock top                    |
         |                           |                       |
     lock nested                     |                       |
         |                           |                       |
         |                           fork------------------->|(held nested lock)
         |                           |                       |
         |                           |                       |
     unlock nested               unlock top              unlock top
                                                             |
                                                             |
                                                 qemuSecuritySetSocketLabel
                                                             |
                                                             |
                                                     lock nested (deadlock)

In this commit, we ensure that the top lock is acquired before the nested lock,
so during fork, it's not possible for another task to acquire the nested lock.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1303031

Signed-off-by: hongmianquan <hongmianquan@xxxxxxxxxxxxx>
---
  src/libvirt_private.syms        |  3 ++-
  src/qemu/qemu_conf.c            |  9 ++++++++-
  src/qemu/qemu_driver.c          | 16 +++++++++-------
  src/qemu/qemu_security.h        |  2 ++
  src/security/security_manager.c | 22 ++++++++++++++++++++++
  src/security/security_manager.h |  2 ++
  6 files changed, 45 insertions(+), 9 deletions(-)

I can get myself convinced there is a deadlock, but I haven't seen it in
a while. Have you encountered this problem recently?

Michal





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux