DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: dev@dpdk.org, david.marchand@redhat.com, chenbox@nvidia.com
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: [PATCH v3 2/6] vhost: fix possible TOCTOU in VDUSE dev creation
Date: Thu, 24 Oct 2024 11:44:02 +0200	[thread overview]
Message-ID: <20241024094406.3826637-3-maxime.coquelin@redhat.com> (raw)
In-Reply-To: <20241024094406.3826637-1-maxime.coquelin@redhat.com>

This patch fixes a possible TOCTOU on the VDUSE
device chardev opening at device creation time.

Coverity issue: 445526
Fixes: da79cc7fda76 ("vhost: add reconnection support to VDUSE")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/vduse.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index 4bb309e441..a98b33dddf 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -546,7 +546,8 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 	else
 		total_queues += 1; /* Includes ctrl queue */
 
-	if (access(path, F_OK) == 0) {
+	dev_fd = open(path, O_RDWR);
+	if (dev_fd >= 0) {
 		VHOST_CONFIG_LOG(name, INFO, "Device already exists, reconnecting...");
 		reconnect = true;
 
@@ -559,7 +560,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 				VHOST_CONFIG_LOG(name, ERR, "Failed to open reconnect file %s (%s)",
 						reconnect_file, strerror(errno));
 			ret = -1;
-			goto out_ctrl_close;
+			goto out_dev_close;
 		}
 
 		reconnect_log = mmap(NULL, sizeof(*reconnect_log), PROT_READ | PROT_WRITE,
@@ -569,7 +570,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 			VHOST_CONFIG_LOG(name, ERR, "Failed to mmap reconnect file %s (%s)",
 					reconnect_file, strerror(errno));
 			ret = -1;
-			goto out_ctrl_close;
+			goto out_dev_close;
 		}
 
 		if (reconnect_log->version != VHOST_RECONNECT_VERSION) {
@@ -593,7 +594,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 			ret = -1;
 			goto out_log_unmap;
 		}
-	} else {
+	} else if (errno == ENOENT) {
 		struct vduse_dev_config *dev_config;
 
 		reco_fd = open(reconnect_file, O_CREAT | O_EXCL | O_RDWR, 0600);
@@ -660,34 +661,39 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 
 		memcpy(&reconnect_log->config, &vnet_config, sizeof(vnet_config));
 		reconnect_log->nr_vrings = total_queues;
-	}
 
-	dev_fd = open(path, O_RDWR);
-	if (dev_fd < 0) {
+		dev_fd = open(path, O_RDWR);
+		if (dev_fd < 0) {
+			VHOST_CONFIG_LOG(name, ERR, "Failed to open newly created device %s: %s",
+					path, strerror(errno));
+			ret = -1;
+			goto out_log_unmap;
+		}
+	} else {
 		VHOST_CONFIG_LOG(name, ERR, "Failed to open device %s: %s",
 				path, strerror(errno));
 		ret = -1;
-		goto out_dev_close;
+		goto out_ctrl_close;
 	}
 
 	ret = fcntl(dev_fd, F_SETFL, O_NONBLOCK);
 	if (ret < 0) {
 		VHOST_CONFIG_LOG(name, ERR, "Failed to set chardev as non-blocking: %s",
 				strerror(errno));
-		goto out_dev_close;
+		goto out_log_unmap;
 	}
 
 	vid = vhost_new_device(&vduse_backend_ops);
 	if (vid < 0) {
 		VHOST_CONFIG_LOG(name, ERR, "Failed to create new Vhost device");
 		ret = -1;
-		goto out_dev_close;
+		goto out_log_unmap;
 	}
 
 	dev = get_device(vid);
 	if (!dev) {
 		ret = -1;
-		goto out_dev_close;
+		goto out_dev_destroy;
 	}
 
 	strncpy(dev->ifname, path, IF_NAME_SZ - 1);
@@ -766,13 +772,13 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 
 out_dev_destroy:
 	vhost_destroy_device(vid);
+out_log_unmap:
+	if (reconnect_log != MAP_FAILED)
+		munmap(reconnect_log, sizeof(*reconnect_log));
 out_dev_close:
 	if (dev_fd >= 0)
 		close(dev_fd);
 	ioctl(control_fd, VDUSE_DESTROY_DEV, name);
-out_log_unmap:
-	if (reconnect_log != MAP_FAILED)
-		munmap(reconnect_log, sizeof(*reconnect_log));
 out_ctrl_close:
 	close(control_fd);
 
-- 
2.46.2


  parent reply	other threads:[~2024-10-24  9:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-24  9:44 [PATCH v3 0/6] VDUSE reconnection fixes and cleanup Maxime Coquelin
2024-10-24  9:44 ` [PATCH v3 1/6] vhost: fix VDUSE device creation error handling Maxime Coquelin
2024-10-24 10:03   ` David Marchand
2024-10-24  9:44 ` Maxime Coquelin [this message]
2024-10-24  9:44 ` [PATCH v3 3/6] vhost: fix VDUSE reconnect device start failure Maxime Coquelin
2024-10-24 10:04   ` David Marchand
2024-10-24  9:44 ` [PATCH v3 4/6] vhost: refactor VDUSE reconnection log mapping Maxime Coquelin
2024-10-24 10:04   ` David Marchand
2024-10-24  9:44 ` [PATCH v3 5/6] vhost: fix and refactor VDUSE reconnect log check Maxime Coquelin
2024-10-24 10:04   ` David Marchand
2024-10-24  9:44 ` [PATCH v3 6/6] vhost: move VDUSE reconnection after device is created Maxime Coquelin
2024-10-24 10:04   ` David Marchand
2024-10-24 11:42 ` [PATCH v3 0/6] VDUSE reconnection fixes and cleanup Maxime Coquelin

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=20241024094406.3826637-3-maxime.coquelin@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=chenbox@nvidia.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).