Skip to content

Commit

Permalink
qmemman: always execute do_balance() under a lock
Browse files Browse the repository at this point in the history
Any memory adjustments must be done while holding a lock, to not
interfere with client request handling. This is critical to prevent
memory just freed for a new VM being re-allocated elsewhere.
The domain_list_changed() function failed to do that - do_balance call
was done after releasing the lock.

It wasn't a problem for a long time because of Python's global interpreter
lock. But Python 3.13 is finally starting to support proper parallel
thread execution, and it revealed this bug.

Fixes QubesOS/qubes-issues#9431
  • Loading branch information
marmarek committed Oct 27, 2024
1 parent 39329c8 commit 2de9eb7
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions qubes/tools/qmemmand.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,19 @@ def domain_list_changed(self, refresh_only=False):
self.watch_token_dict[i])
self.watch_token_dict.pop(i)
system_state.del_domain(i)

if not refresh_only:
try:
system_state.do_balance()
except: # pylint: disable=bare-except
self.log.exception('do_balance() failed')

Check warning on line 136 in qubes/tools/qmemmand.py

View check run for this annotation

Codecov / codecov/patch

qubes/tools/qmemmand.py#L132-L136

Added lines #L132 - L136 were not covered by tests
except: # pylint: disable=bare-except
self.log.exception('Updating domain list failed')
finally:
if got_lock:
global_lock.release()
self.log.debug('global_lock released')

if not refresh_only:
try:
system_state.do_balance()
except: # pylint: disable=bare-except
self.log.exception('do_balance() failed')


def meminfo_changed(self, domain_id):
self.log.debug('meminfo_changed(domain_id={!r})'.format(domain_id))
untrusted_meminfo_key = self.handle.read(
Expand Down

0 comments on commit 2de9eb7

Please sign in to comment.