From 0e3a9bfe1f95c035fdc6308647f06c2f4da0848d Mon Sep 17 00:00:00 2001
From: Norman Feske <norman.feske@genode-labs.com>
Date: Wed, 3 May 2023 12:29:18 +0200
Subject: [PATCH] libc: fix cached ioctl info file access

This patch solves the false-negative error message "failed to open file"
referring to an ioctl info file during an ioctl call. The message is
now avoided by checking for the existence of the file before reading it.

However, the observed symptom uncovered an actual bug that was
introduced in commit "libc vfs: open OSS 'info' file only once" with
the attempt to cache the content of ioctl info files. When called
multiple time for different paths, 'Vfs_plugin::_with_info' would
wrongly return the info from the first call as cached in a local
static variable.

The patch fixes the problem by a new added 'Cached_ioctl_info'
implementation in the scope of the 'Vfs_plugin'.

Issue #4372
Fixes #4852
---
 .../src/lib/libc/internal/vfs_plugin.h        | 33 +++++++++++++++++++
 repos/libports/src/lib/libc/vfs_plugin.cc     | 19 ++++-------
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/repos/libports/src/lib/libc/internal/vfs_plugin.h b/repos/libports/src/lib/libc/internal/vfs_plugin.h
index 554a0c2876..d3202429ca 100644
--- a/repos/libports/src/lib/libc/internal/vfs_plugin.h
+++ b/repos/libports/src/lib/libc/internal/vfs_plugin.h
@@ -93,6 +93,39 @@ class Libc::Vfs_plugin final : public Plugin
 		bool                        const _pipe_configured;
 		Registry<Mmap_entry>              _mmap_registry;
 
+		/*
+		 * Cache the latest info file to accomodate highly frequent 'ioctl'
+		 * calls as observed by the OSS plugin.
+		 */
+		struct Cached_ioctl_info : Noncopyable
+		{
+			Vfs_plugin                  &_vfs_plugin;
+			Constructible<Readonly_file> _file { };
+			Absolute_path                _path { };
+
+			template <typename FN>
+			void with_file(Absolute_path const &path, FN const &fn)
+			{
+				if (!_vfs_plugin._root_dir.constructed()) {
+					warning("Vfs_plugin::_root_dir unexpectedly not constructed");
+					return;
+				}
+
+				Directory &root_dir = *_vfs_plugin._root_dir;
+
+				if (path != _path && root_dir.file_exists(path.string())) {
+					_file.construct(root_dir, path);
+					_path = path;
+				}
+
+				if (path == _path && _file.constructed())
+					fn(*_file);
+			}
+
+			Cached_ioctl_info(Vfs_plugin &vfs_plugin) : _vfs_plugin(vfs_plugin) { }
+
+		} _cached_ioctl_info { *this };
+
 		/**
 		 * Sync a handle
 		 */
diff --git a/repos/libports/src/lib/libc/vfs_plugin.cc b/repos/libports/src/lib/libc/vfs_plugin.cc
index 4de7ed811c..f09dfea7f9 100644
--- a/repos/libports/src/lib/libc/vfs_plugin.cc
+++ b/repos/libports/src/lib/libc/vfs_plugin.cc
@@ -239,26 +239,19 @@ namespace Libc {
 template <typename FN>
 void Libc::Vfs_plugin::_with_info(File_descriptor &fd, FN const &fn)
 {
-	if (!_root_dir.constructed())
-		return;
-
 	Absolute_path path = ioctl_dir(fd);
 	path.append_element("info");
 
-	try {
-		/*
-		 * Opening the info file repeatedly could be too expensive if
-		 * file system servers are part of the VFS, because the directory
-		 * status of the path would be checked at each VFS plugin every
-		 * time. So, we open the file only once.
-		 */
-		static Readonly_file file { *_root_dir, path.string() };
-		static char buffer[4096];
+	_cached_ioctl_info.with_file(path, [&] (Readonly_file const &file) {
+
+		char buffer[4096] { };
+
 		Byte_range_ptr range(buffer,
 		                     min((size_t)(_root_dir->file_size(path.string())),
 		                         sizeof(buffer)));
+
 		with_xml_file_content(file, range, [&] (Xml_node node) { fn(node); });
-	} catch (...) { }
+	});
 }