diff options
author | Liav A <liavalb@gmail.com> | 2023-01-03 22:45:42 +0200 |
---|---|---|
committer | Jelle Raaijmakers <jelle@gmta.nl> | 2023-01-05 23:58:13 +0100 |
commit | a9839d7ac51445af874689c1c293ac597862dcbb (patch) | |
tree | 54cf3b6f6230816fa736f51ba75e2890b20b1e6b /Kernel | |
parent | 76b9d06b199dbcd0e39aa3ea535bbc615e777cec (diff) | |
download | serenity-a9839d7ac51445af874689c1c293ac597862dcbb.zip |
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.
Diffstat (limited to 'Kernel')
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) |