From a7d713952e20ab181d3243a6b5af008bce786973 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Wed, 7 Jul 2021 11:08:41 +0100 Subject: runqemu: Remove potential lock races around tap device handling The qemu tap device handling is potentially race ridden. We pass the fd to the main qemu subprocess which is good as it means the lock is held as long as the qemu process exists. This means we shouldn't unlock it ourselves though, only close the file. We also can't delete the file as we have no idea if qemu is still using it. We could try and obtain an exclusive new lock, then the file would be safe to unlink but it doesn't seem worth it. Also fix the same issue in the port lock code. (From OE-Core rev: 61682a1a8bfe5f7be6e271a4143fa4a02ca5dca9) Signed-off-by: Richard Purdie (cherry picked from commit 2a87bddabf816d09ec801e33972879e6983627eb) Signed-off-by: Anuj Mittal Signed-off-by: Richard Purdie --- scripts/runqemu | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) (limited to 'scripts') diff --git a/scripts/runqemu b/scripts/runqemu index edd17d09c4..c985f4e75a 100755 --- a/scripts/runqemu +++ b/scripts/runqemu @@ -232,9 +232,12 @@ class BaseConfig(object): def release_taplock(self): if self.taplock_descriptor: logger.debug("Releasing lockfile for tap device '%s'" % self.tap) - fcntl.flock(self.taplock_descriptor, fcntl.LOCK_UN) + # We pass the fd to the qemu process and if we unlock here, it would unlock for + # that too. Therefore don't unlock, just close + # fcntl.flock(self.taplock_descriptor, fcntl.LOCK_UN) self.taplock_descriptor.close() - os.remove(self.taplock) + # Removing the file is a potential race, don't do that either + # os.remove(self.taplock) self.taplock_descriptor = None def check_free_port(self, host, port, lockdir): @@ -272,17 +275,23 @@ class BaseConfig(object): def release_portlock(self, lockfile=None): if lockfile != None: - logger.debug("Releasing lockfile '%s'" % lockfile) - fcntl.flock(self.portlocks[lockfile], fcntl.LOCK_UN) - self.portlocks[lockfile].close() - os.remove(lockfile) - del self.portlocks[lockfile] + logger.debug("Releasing lockfile '%s'" % lockfile) + # We pass the fd to the qemu process and if we unlock here, it would unlock for + # that too. Therefore don't unlock, just close + # fcntl.flock(self.portlocks[lockfile], fcntl.LOCK_UN) + self.portlocks[lockfile].close() + # Removing the file is a potential race, don't do that either + # os.remove(lockfile) + del self.portlocks[lockfile] elif len(self.portlocks): for lockfile, descriptor in self.portlocks.items(): logger.debug("Releasing lockfile '%s'" % lockfile) - fcntl.flock(descriptor, fcntl.LOCK_UN) + # We pass the fd to the qemu process and if we unlock here, it would unlock for + # that too. Therefore don't unlock, just close + # fcntl.flock(descriptor, fcntl.LOCK_UN) descriptor.close() - os.remove(lockfile) + # Removing the file is a potential race, don't do that either + # os.remove(lockfile) self.portlocks = {} def get(self, key): -- cgit v1.2.3-54-g00ecf