1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 21:37:35 +00:00

AK: Add trivial structure validation to SharedBuffer

If we're sharing buffers, we only want to share trivial structures
as anything else could potentially share internal pointers, which
most likely is going to cause problems due to different address
spaces.

Fix the GUI::SystemTheme structure, which was not trivial, which
is now caught at compile time.

Fixes #3650
This commit is contained in:
Tom 2020-09-30 20:00:59 -06:00 committed by Andreas Kling
parent 87f20f704c
commit 7399874479
16 changed files with 48 additions and 29 deletions

View file

@ -44,8 +44,21 @@ public:
int shbuf_id() const { return m_shbuf_id; } int shbuf_id() const { return m_shbuf_id; }
void seal(); void seal();
int size() const { return m_size; } int size() const { return m_size; }
void* data() { return m_data; }
const void* data() const { return m_data; } template<typename T>
T* data()
{
static_assert(IsVoid<T>::value || is_trivial<T>());
return (T*)m_data;
}
template<typename T>
const T* data() const
{
static_assert(IsVoid<T>::value || is_trivial<T>());
return (const T*)m_data;
}
void set_volatile(); void set_volatile();
[[nodiscard]] bool set_nonvolatile(); [[nodiscard]] bool set_nonvolatile();

View file

@ -120,7 +120,7 @@ public:
const Sample* samples() const { return (const Sample*)data(); } const Sample* samples() const { return (const Sample*)data(); }
int sample_count() const { return m_sample_count; } int sample_count() const { return m_sample_count; }
const void* data() const { return m_buffer->data(); } const void* data() const { return m_buffer->data<void>(); }
int size_in_bytes() const { return m_sample_count * (int)sizeof(Sample); } int size_in_bytes() const { return m_sample_count * (int)sizeof(Sample); }
int shbuf_id() const { return m_buffer->shbuf_id(); } int shbuf_id() const { return m_buffer->shbuf_id(); }
SharedBuffer& shared_buffer() { return *m_buffer; } SharedBuffer& shared_buffer() { return *m_buffer; }
@ -130,7 +130,7 @@ private:
: m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(Sample))) : m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(Sample)))
, m_sample_count(samples.size()) , m_sample_count(samples.size())
{ {
memcpy(m_buffer->data(), samples.data(), samples.size() * sizeof(Sample)); memcpy(m_buffer->data<void>(), samples.data(), samples.size() * sizeof(Sample));
} }
explicit Buffer(NonnullRefPtr<SharedBuffer>&& buffer, int sample_count) explicit Buffer(NonnullRefPtr<SharedBuffer>&& buffer, int sample_count)

View file

@ -90,7 +90,7 @@ Clipboard::DataAndType Clipboard::data_and_type() const
dbgprintf("GUI::Clipboard::data() clipping contents size is greater than shared buffer size\n"); dbgprintf("GUI::Clipboard::data() clipping contents size is greater than shared buffer size\n");
return {}; return {};
} }
auto data = ByteBuffer::copy(shared_buffer->data(), response->data_size()); auto data = ByteBuffer::copy(shared_buffer->data<void>(), response->data_size());
auto type = response->mime_type(); auto type = response->mime_type();
auto metadata = response->metadata().entries(); auto metadata = response->metadata().entries();
return { data, type, metadata }; return { data, type, metadata };
@ -104,7 +104,7 @@ void Clipboard::set_data(ReadonlyBytes data, const String& type, const HashMap<S
return; return;
} }
if (!data.is_empty()) if (!data.is_empty())
memcpy(shared_buffer->data(), data.data(), data.size()); memcpy(shared_buffer->data<void>(), data.data(), data.size());
shared_buffer->seal(); shared_buffer->seal();
shared_buffer->share_with(connection().server_pid()); shared_buffer->share_with(connection().server_pid());

View file

@ -118,7 +118,7 @@ static int ensure_realized_icon(IconContainerType& container)
auto shared_buffer = SharedBuffer::create_with_size(container.icon()->size_in_bytes()); auto shared_buffer = SharedBuffer::create_with_size(container.icon()->size_in_bytes());
ASSERT(shared_buffer); ASSERT(shared_buffer);
auto shared_icon = Gfx::Bitmap::create_with_shared_buffer(Gfx::BitmapFormat::RGBA32, *shared_buffer, container.icon()->size()); auto shared_icon = Gfx::Bitmap::create_with_shared_buffer(Gfx::BitmapFormat::RGBA32, *shared_buffer, container.icon()->size());
memcpy(shared_buffer->data(), container.icon()->scanline_u8(0), container.icon()->size_in_bytes()); memcpy(shared_buffer->template data<u8>(), container.icon()->scanline_u8(0), container.icon()->size_in_bytes());
shared_buffer->seal(); shared_buffer->seal();
shared_buffer->share_with(WindowServerConnection::the().server_pid()); shared_buffer->share_with(WindowServerConnection::the().server_pid());
container.set_icon(shared_icon); container.set_icon(shared_icon);

View file

@ -169,7 +169,7 @@ RefPtr<Bitmap> Bitmap::create_with_shared_buffer(BitmapFormat format, NonnullRef
Bitmap::Bitmap(BitmapFormat format, NonnullRefPtr<SharedBuffer>&& shared_buffer, const IntSize& size, const Vector<RGBA32>& palette) Bitmap::Bitmap(BitmapFormat format, NonnullRefPtr<SharedBuffer>&& shared_buffer, const IntSize& size, const Vector<RGBA32>& palette)
: m_size(size) : m_size(size)
, m_data(shared_buffer->data()) , m_data(shared_buffer->data<void>())
, m_pitch(minimum_pitch(size.width(), format)) , m_pitch(minimum_pitch(size.width(), format))
, m_format(format) , m_format(format)
, m_shared_buffer(move(shared_buffer)) , m_shared_buffer(move(shared_buffer))
@ -255,7 +255,7 @@ RefPtr<Bitmap> Bitmap::to_bitmap_backed_by_shared_buffer() const
auto bitmap = Bitmap::create_with_shared_buffer(m_format, *buffer, m_size, palette_to_vector()); auto bitmap = Bitmap::create_with_shared_buffer(m_format, *buffer, m_size, palette_to_vector());
if (!bitmap) if (!bitmap)
return nullptr; return nullptr;
memcpy(buffer->data(), scanline(0), size_in_bytes()); memcpy(buffer->data<void>(), scanline(0), size_in_bytes());
return bitmap; return bitmap;
} }

View file

@ -52,13 +52,13 @@ Palette::~Palette()
const SystemTheme& PaletteImpl::theme() const const SystemTheme& PaletteImpl::theme() const
{ {
return *(const SystemTheme*)m_theme_buffer->data(); return *m_theme_buffer->data<SystemTheme>();
} }
Color PaletteImpl::color(ColorRole role) const Color PaletteImpl::color(ColorRole role) const
{ {
ASSERT((int)role < (int)ColorRole::__Count); ASSERT((int)role < (int)ColorRole::__Count);
return theme().color[(int)role]; return Color::from_rgba(theme().color[(int)role]);
} }
int PaletteImpl::metric(MetricRole role) const int PaletteImpl::metric(MetricRole role) const
@ -76,7 +76,7 @@ String PaletteImpl::path(PathRole role) const
NonnullRefPtr<PaletteImpl> PaletteImpl::clone() const NonnullRefPtr<PaletteImpl> PaletteImpl::clone() const
{ {
auto new_theme_buffer = SharedBuffer::create_with_size(m_theme_buffer->size()); auto new_theme_buffer = SharedBuffer::create_with_size(m_theme_buffer->size());
memcpy(new_theme_buffer->data(), m_theme_buffer->data(), m_theme_buffer->size()); memcpy(new_theme_buffer->data<SystemTheme>(), &theme(), m_theme_buffer->size());
return adopt(*new PaletteImpl(*new_theme_buffer)); return adopt(*new PaletteImpl(*new_theme_buffer));
} }
@ -85,7 +85,7 @@ void Palette::set_color(ColorRole role, Color color)
if (m_impl->ref_count() != 1) if (m_impl->ref_count() != 1)
m_impl = m_impl->clone(); m_impl = m_impl->clone();
auto& theme = const_cast<SystemTheme&>(impl().theme()); auto& theme = const_cast<SystemTheme&>(impl().theme());
theme.color[(int)role] = color; theme.color[(int)role] = color.value();
} }
void Palette::set_metric(MetricRole role, int value) void Palette::set_metric(MetricRole role, int value)
@ -101,7 +101,8 @@ void Palette::set_path(PathRole role, String path)
if (m_impl->ref_count() != 1) if (m_impl->ref_count() != 1)
m_impl = m_impl->clone(); m_impl = m_impl->clone();
auto& theme = const_cast<SystemTheme&>(impl().theme()); auto& theme = const_cast<SystemTheme&>(impl().theme());
theme.path[(int)role] = path; memcpy(theme.path[(int)role], path.characters(), min(path.length() + 1, sizeof(theme.path[(int)role])));
theme.path[(int)role][sizeof(theme.path[(int)role]) - 1] = '\0';
} }
PaletteImpl::~PaletteImpl() PaletteImpl::~PaletteImpl()

View file

@ -27,6 +27,7 @@
#include <AK/SharedBuffer.h> #include <AK/SharedBuffer.h>
#include <LibCore/ConfigFile.h> #include <LibCore/ConfigFile.h>
#include <LibGfx/SystemTheme.h> #include <LibGfx/SystemTheme.h>
#include <string.h>
namespace Gfx { namespace Gfx {
@ -49,7 +50,7 @@ int current_system_theme_buffer_id()
void set_system_theme(SharedBuffer& buffer) void set_system_theme(SharedBuffer& buffer)
{ {
theme_buffer = buffer; theme_buffer = buffer;
theme_page = (SystemTheme*)theme_buffer->data(); theme_page = theme_buffer->data<SystemTheme>();
} }
RefPtr<SharedBuffer> load_system_theme(const String& path) RefPtr<SharedBuffer> load_system_theme(const String& path)
@ -57,7 +58,7 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
auto file = Core::ConfigFile::open(path); auto file = Core::ConfigFile::open(path);
auto buffer = SharedBuffer::create_with_size(sizeof(SystemTheme)); auto buffer = SharedBuffer::create_with_size(sizeof(SystemTheme));
auto* data = (SystemTheme*)buffer->data(); auto* data = buffer->data<SystemTheme>();
auto get_color = [&](auto& name) { auto get_color = [&](auto& name) {
auto color_string = file->read_entry("Colors", name); auto color_string = file->read_entry("Colors", name);
@ -100,7 +101,7 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
#undef __ENUMERATE_COLOR_ROLE #undef __ENUMERATE_COLOR_ROLE
#define __ENUMERATE_COLOR_ROLE(role) \ #define __ENUMERATE_COLOR_ROLE(role) \
data->color[(int)ColorRole::role] = get_color(#role); data->color[(int)ColorRole::role] = get_color(#role).value();
ENUMERATE_COLOR_ROLES(__ENUMERATE_COLOR_ROLE) ENUMERATE_COLOR_ROLES(__ENUMERATE_COLOR_ROLE)
#undef __ENUMERATE_COLOR_ROLE #undef __ENUMERATE_COLOR_ROLE
@ -111,8 +112,12 @@ RefPtr<SharedBuffer> load_system_theme(const String& path)
DO_METRIC(TitleButtonWidth); DO_METRIC(TitleButtonWidth);
DO_METRIC(TitleButtonHeight); DO_METRIC(TitleButtonHeight);
#define DO_PATH(x) \ #define DO_PATH(x) \
data->path[(int)PathRole::x] = get_path(#x, (int)PathRole::x) do { \
auto path = get_path(#x, (int)PathRole::x); \
memcpy(data->path[(int)PathRole::x], path, min(strlen(path) + 1, sizeof(data->path[(int)PathRole::x]))); \
data->path[(int)PathRole::x][sizeof(data->path[(int)PathRole::x]) - 1] = '\0'; \
} while (0)
DO_PATH(TitleButtonIcons); DO_PATH(TitleButtonIcons);

View file

@ -143,9 +143,9 @@ enum class PathRole {
}; };
struct SystemTheme { struct SystemTheme {
Color color[(int)ColorRole::__Count]; RGBA32 color[(int)ColorRole::__Count];
int metric[(int)MetricRole::__Count]; int metric[(int)MetricRole::__Count];
String path[(int)PathRole::__Count]; char path[(int)PathRole::__Count][256]; // TODO: PATH_MAX?
}; };
const SystemTheme& current_system_theme(); const SystemTheme& current_system_theme();

View file

@ -57,7 +57,7 @@ RefPtr<Gfx::Bitmap> Client::decode_image(const ByteBuffer& encoded_data)
return nullptr; return nullptr;
} }
memcpy(encoded_buffer->data(), encoded_data.data(), encoded_data.size()); memcpy(encoded_buffer->data<void>(), encoded_data.data(), encoded_data.size());
encoded_buffer->seal(); encoded_buffer->seal();
encoded_buffer->share_with(server_pid()); encoded_buffer->share_with(server_pid());

View file

@ -50,7 +50,7 @@ void Download::did_finish(Badge<Client>, bool success, Optional<u32> status_code
RefPtr<SharedBuffer> shared_buffer; RefPtr<SharedBuffer> shared_buffer;
if (success && shbuf_id != -1) { if (success && shbuf_id != -1) {
shared_buffer = SharedBuffer::create_from_shbuf_id(shbuf_id); shared_buffer = SharedBuffer::create_from_shbuf_id(shbuf_id);
payload = ByteBuffer::wrap(shared_buffer->data(), total_size); payload = ByteBuffer::wrap(shared_buffer->data<void>(), total_size);
} }
// FIXME: It's a bit silly that we copy the response headers here just so we can move them into a HashMap with different traits. // FIXME: It's a bit silly that we copy the response headers here just so we can move them into a HashMap with different traits.

View file

@ -83,7 +83,7 @@ OwnPtr<Messages::ClipboardServer::GetClipboardDataResponse> ClientConnection::ha
// It would be even nicer if a SharedBuffer could have an arbitrary number of clients.. // It would be even nicer if a SharedBuffer could have an arbitrary number of clients..
RefPtr<SharedBuffer> shared_buffer = SharedBuffer::create_with_size(storage.data_size()); RefPtr<SharedBuffer> shared_buffer = SharedBuffer::create_with_size(storage.data_size());
ASSERT(shared_buffer); ASSERT(shared_buffer);
memcpy(shared_buffer->data(), storage.data(), storage.data_size()); memcpy(shared_buffer->data<void>(), storage.data(), storage.data_size());
shared_buffer->seal(); shared_buffer->seal();
shared_buffer->share_with(client_pid()); shared_buffer->share_with(client_pid());
shbuf_id = shared_buffer->shbuf_id(); shbuf_id = shared_buffer->shbuf_id();

View file

@ -46,7 +46,7 @@ Storage::~Storage()
void Storage::set_data(NonnullRefPtr<SharedBuffer> data, size_t data_size, const String& mime_type, const HashMap<String, String>& metadata) void Storage::set_data(NonnullRefPtr<SharedBuffer> data, size_t data_size, const String& mime_type, const HashMap<String, String>& metadata)
{ {
dbg() << "Storage::set_data <- [" << mime_type << "] " << data->data() << " (" << data_size << " bytes)"; dbg() << "Storage::set_data <- [" << mime_type << "] " << data->data<void>() << " (" << data_size << " bytes)";
for (auto& it : metadata) { for (auto& it : metadata) {
dbg() << " " << it.key << ": " << it.value; dbg() << " " << it.key << ": " << it.value;
} }

View file

@ -47,7 +47,7 @@ public:
{ {
if (!has_data()) if (!has_data())
return nullptr; return nullptr;
return static_cast<const u8*>(m_shared_buffer->data()); return m_shared_buffer->data<u8>();
} }
size_t data_size() const size_t data_size() const

View file

@ -79,7 +79,7 @@ OwnPtr<Messages::ImageDecoderServer::DecodeImageResponse> ClientConnection::hand
dbg() << "Trying to decode " << message.encoded_size() << " bytes of image(?) data in shbuf_id=" << message.encoded_shbuf_id() << " (shbuf size: " << encoded_buffer->size() << ")"; dbg() << "Trying to decode " << message.encoded_size() << " bytes of image(?) data in shbuf_id=" << message.encoded_shbuf_id() << " (shbuf size: " << encoded_buffer->size() << ")";
#endif #endif
auto decoder = Gfx::ImageDecoder::create((const u8*)encoded_buffer->data(), message.encoded_size()); auto decoder = Gfx::ImageDecoder::create(encoded_buffer->data<u8>(), message.encoded_size());
auto bitmap = decoder->bitmap(); auto bitmap = decoder->bitmap();
if (!bitmap) { if (!bitmap) {

View file

@ -91,7 +91,7 @@ void ClientConnection::did_finish_download(Badge<Download>, Download& download,
RefPtr<SharedBuffer> buffer; RefPtr<SharedBuffer> buffer;
if (success && download.payload().size() > 0 && !download.payload().is_null()) { if (success && download.payload().size() > 0 && !download.payload().is_null()) {
buffer = SharedBuffer::create_with_size(download.payload().size()); buffer = SharedBuffer::create_with_size(download.payload().size());
memcpy(buffer->data(), download.payload().data(), download.payload().size()); memcpy(buffer->data<void>(), download.payload().data(), download.payload().size());
buffer->seal(); buffer->seal();
buffer->share_with(client_pid()); buffer->share_with(client_pid());
m_shared_buffers.set(buffer->shbuf_id(), buffer); m_shared_buffers.set(buffer->shbuf_id(), buffer);

View file

@ -50,7 +50,7 @@ void PageHost::setup_palette()
{ {
// FIXME: Get the proper palette from our peer somehow // FIXME: Get the proper palette from our peer somehow
auto buffer = SharedBuffer::create_with_size(sizeof(Gfx::SystemTheme)); auto buffer = SharedBuffer::create_with_size(sizeof(Gfx::SystemTheme));
auto* theme = (Gfx::SystemTheme*)buffer->data(); auto* theme = buffer->data<Gfx::SystemTheme>();
theme->color[(int)Gfx::ColorRole::Window] = Color::Magenta; theme->color[(int)Gfx::ColorRole::Window] = Color::Magenta;
theme->color[(int)Gfx::ColorRole::WindowText] = Color::Cyan; theme->color[(int)Gfx::ColorRole::WindowText] = Color::Cyan;
m_palette_impl = Gfx::PaletteImpl::create_with_shared_buffer(*buffer); m_palette_impl = Gfx::PaletteImpl::create_with_shared_buffer(*buffer);