diff options
author | Richard Purdie <richard.purdie@linuxfoundation.org> | 2021-07-07 11:08:41 +0100 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2021-07-08 14:58:28 +0100 |
commit | a16d7d2ec63f911657330dd1df9c8cb069bbd5ee (patch) | |
tree | 0837a328f487521c4abe218903ced20f7f58de5e /scripts | |
parent | bdc5f4e892f36d90737e87192ab3f2c30d8dbdd9 (diff) | |
download | poky-a16d7d2ec63f911657330dd1df9c8cb069bbd5ee.tar.gz |
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: 2a87bddabf816d09ec801e33972879e6983627eb)
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Diffstat (limited to 'scripts')
-rwxr-xr-x | scripts/runqemu | 27 |
1 files changed, 18 insertions, 9 deletions
diff --git a/scripts/runqemu b/scripts/runqemu index 1f332ef525..2914f15d06 100755 --- a/scripts/runqemu +++ b/scripts/runqemu | |||
@@ -233,9 +233,12 @@ class BaseConfig(object): | |||
233 | def release_taplock(self): | 233 | def release_taplock(self): |
234 | if self.taplock_descriptor: | 234 | if self.taplock_descriptor: |
235 | logger.debug("Releasing lockfile for tap device '%s'" % self.tap) | 235 | logger.debug("Releasing lockfile for tap device '%s'" % self.tap) |
236 | fcntl.flock(self.taplock_descriptor, fcntl.LOCK_UN) | 236 | # We pass the fd to the qemu process and if we unlock here, it would unlock for |
237 | # that too. Therefore don't unlock, just close | ||
238 | # fcntl.flock(self.taplock_descriptor, fcntl.LOCK_UN) | ||
237 | self.taplock_descriptor.close() | 239 | self.taplock_descriptor.close() |
238 | os.remove(self.taplock) | 240 | # Removing the file is a potential race, don't do that either |
241 | # os.remove(self.taplock) | ||
239 | self.taplock_descriptor = None | 242 | self.taplock_descriptor = None |
240 | 243 | ||
241 | def check_free_port(self, host, port, lockdir): | 244 | def check_free_port(self, host, port, lockdir): |
@@ -273,17 +276,23 @@ class BaseConfig(object): | |||
273 | 276 | ||
274 | def release_portlock(self, lockfile=None): | 277 | def release_portlock(self, lockfile=None): |
275 | if lockfile != None: | 278 | if lockfile != None: |
276 | logger.debug("Releasing lockfile '%s'" % lockfile) | 279 | logger.debug("Releasing lockfile '%s'" % lockfile) |
277 | fcntl.flock(self.portlocks[lockfile], fcntl.LOCK_UN) | 280 | # We pass the fd to the qemu process and if we unlock here, it would unlock for |
278 | self.portlocks[lockfile].close() | 281 | # that too. Therefore don't unlock, just close |
279 | os.remove(lockfile) | 282 | # fcntl.flock(self.portlocks[lockfile], fcntl.LOCK_UN) |
280 | del self.portlocks[lockfile] | 283 | self.portlocks[lockfile].close() |
284 | # Removing the file is a potential race, don't do that either | ||
285 | # os.remove(lockfile) | ||
286 | del self.portlocks[lockfile] | ||
281 | elif len(self.portlocks): | 287 | elif len(self.portlocks): |
282 | for lockfile, descriptor in self.portlocks.items(): | 288 | for lockfile, descriptor in self.portlocks.items(): |
283 | logger.debug("Releasing lockfile '%s'" % lockfile) | 289 | logger.debug("Releasing lockfile '%s'" % lockfile) |
284 | fcntl.flock(descriptor, fcntl.LOCK_UN) | 290 | # We pass the fd to the qemu process and if we unlock here, it would unlock for |
291 | # that too. Therefore don't unlock, just close | ||
292 | # fcntl.flock(descriptor, fcntl.LOCK_UN) | ||
285 | descriptor.close() | 293 | descriptor.close() |
286 | os.remove(lockfile) | 294 | # Removing the file is a potential race, don't do that either |
295 | # os.remove(lockfile) | ||
287 | self.portlocks = {} | 296 | self.portlocks = {} |
288 | 297 | ||
289 | def get(self, key): | 298 | def get(self, key): |