All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Anuj Mittal" <anuj.mittal@intel.com>
To: openembedded-core@lists.openembedded.org
Subject: [hardknott][PATCH 15/28] runqemu: Remove potential lock races around tap device handling
Date: Fri, 16 Jul 2021 10:42:13 +0800	[thread overview]
Message-ID: <61682a1a8bfe5f7be6e271a4143fa4a02ca5dca9.1626403078.git.anuj.mittal@intel.com> (raw)
In-Reply-To: <cover.1626403078.git.anuj.mittal@intel.com>

From: Richard Purdie <richard.purdie@linuxfoundation.org>

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.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 2a87bddabf816d09ec801e33972879e6983627eb)
Signed-off-by: Anuj Mittal <anuj.mittal@intel.com>
---
 scripts/runqemu | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

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):
-- 
2.31.1


  parent reply	other threads:[~2021-07-16  2:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16  2:41 [hardknott][PATCH 00/28] review request Anuj Mittal
2021-07-16  2:41 ` [hardknott][PATCH 01/28] oeqa/selftest/runcmd: Tweal test timeouts Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 02/28] sstate/staging: Handle directory creation race issue Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 03/28] devtool: deploy-target: Fix preserving attributes when using --strip Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 04/28] oeqa/selftest/archiver: Allow tests to ignore empty directories Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 05/28] openssh: Remove temporary keys before generating new ones Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 06/28] linux-yocto/5.10: update to v5.10.47 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 07/28] linux-yocto/5.4: update to v5.4.129 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 08/28] linux-yocto/5.10: scsi-debug needs scsi-disk Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 09/28] linux-firmware: Package RSI 911x WiFi firmware Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 10/28] libconvert-asn1-perl: fix CVE-2013-7488 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 11/28] busybox: upgrade 1.33.0 -> 1.33.1 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 12/28] perl: correct libpth and glibpth Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 13/28] rxvt-unicode: fix CVE-2021-33477 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 14/28] binutils: Fix CVE-2021-20197 Anuj Mittal
2021-07-16  2:42 ` Anuj Mittal [this message]
2021-07-16  2:42 ` [hardknott][PATCH 16/28] glibc-testsuite: Fix build failures when directly running recipe Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 17/28] boost-build-native: workaround one rarely hang problem on fedora34 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 18/28] linux-yocto-dev: base AUTOREV on specified version Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 19/28] go: upgrade 1.16.3 -> 1.16.4 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 20/28] go: upgrade 1.16.4 -> 1.16.5 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 21/28] curl: Fix CVE-2021-22898 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 22/28] curl: Fix CVE-2021-22897 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 23/28] oeqa/selftest/multiprocesslauch: Fix test race Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 24/28] dwarfsrcfiles: Avoid races over debug-link files Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 25/28] kernel-devsrc: fix scripts/prepare for ARM64 Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 26/28] kernel-devsrc: fix scripts prepare for powerpc Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 27/28] busybox: add tmpdir option into mktemp applet Anuj Mittal
2021-07-16  2:42 ` [hardknott][PATCH 28/28] xserver-xorg: Fix builds without glx Anuj Mittal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=61682a1a8bfe5f7be6e271a4143fa4a02ca5dca9.1626403078.git.anuj.mittal@intel.com \
    --to=anuj.mittal@intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.