From a9839d7ac51445af874689c1c293ac597862dcbb Mon Sep 17 00:00:00 2001
From: Liav A <liavalb@gmail.com>
Date: Tue, 3 Jan 2023 22:45:42 +0200
Subject: [PATCH] Kernel/SysFS: Don't refresh/set-values inside the Jail
 spinlock scope

Only do so after a brief check if we are in a Jail or not. This fixes
SMP, because apparently it is crashing when calling try_generate()
from the SysFSGlobalInformation::refresh_data method, so the fix for
this is to simply not do that inside the Process' Jail spinlock scope,
because otherwise we will simply have a possible flow of taking
multiple conflicting Spinlocks (in the wrong order multiple times), for
the SysFSOverallProcesses generation code:
Process::current().jail(), and then Process::for_each_in_same_jail being
called, we take Process::all_instances(), and Process::current().jail()
again.
Therefore, we should at the very least eliminate the first taking of the
Process::current().jail() spinlock, in the refresh_data method of the
SysFSGlobalInformation class.
---
 .../Subsystems/Kernel/GlobalInformation.cpp   |  2 +-
 .../Kernel/Variables/BooleanVariable.cpp      | 22 ++++++++++---------
 .../Kernel/Variables/StringVariable.cpp       |  9 ++++----
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/Kernel/FileSystem/SysFS/Subsystems/Kernel/GlobalInformation.cpp b/Kernel/FileSystem/SysFS/Subsystems/Kernel/GlobalInformation.cpp
index 20800dab8f..dbd64154f0 100644
--- a/Kernel/FileSystem/SysFS/Subsystems/Kernel/GlobalInformation.cpp
+++ b/Kernel/FileSystem/SysFS/Subsystems/Kernel/GlobalInformation.cpp
@@ -55,9 +55,9 @@ ErrorOr<void> SysFSGlobalInformation::refresh_data(OpenFileDescription& descript
     TRY(Process::current().jail().with([&](auto& my_jail) -> ErrorOr<void> {
         if (my_jail && !is_readable_by_jailed_processes())
             return Error::from_errno(EPERM);
-        TRY(const_cast<SysFSGlobalInformation&>(*this).try_generate(builder));
         return {};
     }));
+    TRY(const_cast<SysFSGlobalInformation&>(*this).try_generate(builder));
     auto& typed_cached_data = static_cast<SysFSInodeData&>(*cached_data);
     typed_cached_data.buffer = builder.build();
     if (!typed_cached_data.buffer)
diff --git a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/BooleanVariable.cpp b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/BooleanVariable.cpp
index 4f7769e9a9..7486e8f2c5 100644
--- a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/BooleanVariable.cpp
+++ b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/BooleanVariable.cpp
@@ -23,20 +23,22 @@ ErrorOr<size_t> SysFSSystemBooleanVariable::write_bytes(off_t, size_t count, Use
     char value = 0;
     TRY(buffer.read(&value, 1));
 
-    return Process::current().jail().with([&](auto& my_jail) -> ErrorOr<size_t> {
+    TRY(Process::current().jail().with([&](auto& my_jail) -> ErrorOr<void> {
         // Note: If we are in a jail, don't let the current process to change the variable.
         if (my_jail)
             return Error::from_errno(EPERM);
-        if (count != 1)
-            return Error::from_errno(EINVAL);
-        if (value == '0')
-            set_value(false);
-        else if (value == '1')
-            set_value(true);
-        else
-            return Error::from_errno(EINVAL);
+        return {};
+    }));
+    if (count != 1)
+        return Error::from_errno(EINVAL);
+    if (value == '0') {
+        set_value(false);
         return 1;
-    });
+    } else if (value == '1') {
+        set_value(true);
+        return 1;
+    }
+    return Error::from_errno(EINVAL);
 }
 
 ErrorOr<void> SysFSSystemBooleanVariable::truncate(u64 size)
diff --git a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp
index b4aa97ba20..8064589aab 100644
--- a/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp
+++ b/Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp
@@ -25,13 +25,14 @@ ErrorOr<size_t> SysFSSystemStringVariable::write_bytes(off_t, size_t count, User
     auto new_value = TRY(KString::try_create_uninitialized(count, value));
     TRY(buffer.read(value, count));
     auto new_value_without_possible_newlines = TRY(KString::try_create(new_value->view().trim("\n"sv)));
-    return Process::current().jail().with([&](auto& my_jail) -> ErrorOr<size_t> {
+    TRY(Process::current().jail().with([&](auto& my_jail) -> ErrorOr<void> {
         // Note: If we are in a jail, don't let the current process to change the variable.
         if (my_jail)
             return Error::from_errno(EPERM);
-        set_value(move(new_value_without_possible_newlines));
-        return count;
-    });
+        return {};
+    }));
+    set_value(move(new_value_without_possible_newlines));
+    return count;
 }
 
 ErrorOr<void> SysFSSystemStringVariable::truncate(u64 size)