Skip to content

Commit

Permalink
Everywhere: Use OOM-safe ByteBuffer APIs where possible
Browse files Browse the repository at this point in the history
If we can easily communicate failure, let's avoid asserting and report
failure instead.
  • Loading branch information
alimpfard authored and awesomekling committed Sep 5, 2021
1 parent 6606993 commit 3a9f00c
Show file tree
Hide file tree
Showing 22 changed files with 135 additions and 67 deletions.
18 changes: 11 additions & 7 deletions AK/StringBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@

namespace AK {

inline void StringBuilder::will_append(size_t size)
inline bool StringBuilder::will_append(size_t size)
{
Checked<size_t> needed_capacity = m_buffer.size();
needed_capacity += size;
VERIFY(!needed_capacity.has_overflow());
// Prefer to completely use the existing capacity first
if (needed_capacity <= m_buffer.capacity())
return;
return true;
Checked<size_t> expanded_capacity = needed_capacity;
expanded_capacity *= 2;
VERIFY(!expanded_capacity.has_overflow());
m_buffer.ensure_capacity(expanded_capacity.value());
return m_buffer.try_ensure_capacity(expanded_capacity.value());
}

StringBuilder::StringBuilder(size_t initial_capacity)
Expand All @@ -40,8 +40,10 @@ void StringBuilder::append(StringView const& str)
{
if (str.is_empty())
return;
will_append(str.length());
m_buffer.append(str.characters_without_null_termination(), str.length());
auto ok = will_append(str.length());
VERIFY(ok);
ok = m_buffer.try_append(str.characters_without_null_termination(), str.length());
VERIFY(ok);
}

void StringBuilder::append(char const* characters, size_t length)
Expand All @@ -51,8 +53,10 @@ void StringBuilder::append(char const* characters, size_t length)

void StringBuilder::append(char ch)
{
will_append(1);
m_buffer.append(&ch, 1);
auto ok = will_append(1);
VERIFY(ok);
ok = m_buffer.try_append(&ch, 1);
VERIFY(ok);
}

void StringBuilder::appendvf(char const* fmt, va_list ap)
Expand Down
2 changes: 1 addition & 1 deletion AK/StringBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class StringBuilder {
}

private:
void will_append(size_t);
bool will_append(size_t);
u8* data() { return m_buffer.data(); }
u8 const* data() const { return m_buffer.data(); }

Expand Down
79 changes: 52 additions & 27 deletions Kernel/Coredump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,15 @@ KResult Coredump::write_notes_segment(ByteBuffer& notes_segment)
return KSuccess;
}

ByteBuffer Coredump::create_notes_process_data() const
KResultOr<ByteBuffer> Coredump::create_notes_process_data() const
{
ByteBuffer process_data;

ELF::Core::ProcessInfo info {};
info.header.type = ELF::Core::NotesEntryHeader::Type::ProcessInfo;
process_data.append((void*)&info, sizeof(info));
auto ok = process_data.try_append((void*)&info, sizeof(info));
if (!ok)
return ENOMEM;

StringBuilder builder;
{
Expand All @@ -227,68 +229,69 @@ ByteBuffer Coredump::create_notes_process_data() const
}

builder.append(0);
process_data.append(builder.string_view().characters_without_null_termination(), builder.length());
ok = process_data.try_append(builder.string_view().characters_without_null_termination(), builder.length());
if (!ok)
return ENOMEM;

return process_data;
}

ByteBuffer Coredump::create_notes_threads_data() const
KResultOr<ByteBuffer> Coredump::create_notes_threads_data() const
{
ByteBuffer threads_data;

for (auto& thread : m_process->threads_for_coredump({})) {
ByteBuffer entry_buff;

ELF::Core::ThreadInfo info {};
info.header.type = ELF::Core::NotesEntryHeader::Type::ThreadInfo;
info.tid = thread.tid().value();

if (thread.current_trap())
copy_kernel_registers_into_ptrace_registers(info.regs, thread.get_register_dump_from_stack());

entry_buff.append((void*)&info, sizeof(info));

threads_data += entry_buff;
if (!threads_data.try_append(&info, sizeof(info)))
return ENOMEM;
}
return threads_data;
}

ByteBuffer Coredump::create_notes_regions_data() const
KResultOr<ByteBuffer> Coredump::create_notes_regions_data() const
{
ByteBuffer regions_data;
size_t region_index = 0;
for (auto& region : m_process->address_space().regions()) {

ByteBuffer memory_region_info_buffer;
ELF::Core::MemoryRegionInfo info {};
info.header.type = ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo;

info.region_start = region->vaddr().get();
info.region_end = region->vaddr().offset(region->size()).get();
info.program_header_index = region_index++;

memory_region_info_buffer.append((void*)&info, sizeof(info));
if (!regions_data.try_ensure_capacity(regions_data.size() + sizeof(info) + region->name().length() + 1))
return ENOMEM;

auto ok = regions_data.try_append((void*)&info, sizeof(info));
// NOTE: The region name *is* null-terminated, so the following is ok:
auto name = region->name();
if (name.is_empty()) {
char null_terminator = '\0';
memory_region_info_buffer.append(&null_terminator, 1);
ok = ok && regions_data.try_append(&null_terminator, 1);
} else {
memory_region_info_buffer.append(name.characters_without_null_termination(), name.length() + 1);
ok = ok && regions_data.try_append(name.characters_without_null_termination(), name.length() + 1);
}

regions_data += memory_region_info_buffer;
VERIFY(ok);
}
return regions_data;
}

ByteBuffer Coredump::create_notes_metadata_data() const
KResultOr<ByteBuffer> Coredump::create_notes_metadata_data() const
{
ByteBuffer metadata_data;

ELF::Core::Metadata metadata {};
metadata.header.type = ELF::Core::NotesEntryHeader::Type::Metadata;
metadata_data.append((void*)&metadata, sizeof(metadata));
auto ok = metadata_data.try_append((void*)&metadata, sizeof(metadata));
if (!ok)
return ENOMEM;

StringBuilder builder;
{
Expand All @@ -298,23 +301,41 @@ ByteBuffer Coredump::create_notes_metadata_data() const
});
}
builder.append(0);
metadata_data.append(builder.string_view().characters_without_null_termination(), builder.length());
ok = metadata_data.try_append(builder.string_view().characters_without_null_termination(), builder.length());
if (!ok)
return ENOMEM;

return metadata_data;
}

ByteBuffer Coredump::create_notes_segment_data() const
KResultOr<ByteBuffer> Coredump::create_notes_segment_data() const
{
ByteBuffer notes_buffer;

notes_buffer += create_notes_process_data();
notes_buffer += create_notes_threads_data();
notes_buffer += create_notes_regions_data();
notes_buffer += create_notes_metadata_data();
if (auto result = create_notes_process_data(); result.is_error())
return result;
else if (!notes_buffer.try_append(result.value()))
return ENOMEM;

if (auto result = create_notes_threads_data(); result.is_error())
return result;
else if (!notes_buffer.try_append(result.value()))
return ENOMEM;

if (auto result = create_notes_regions_data(); result.is_error())
return result;
else if (!notes_buffer.try_append(result.value()))
return ENOMEM;

if (auto result = create_notes_metadata_data(); result.is_error())
return result;
else if (!notes_buffer.try_append(result.value()))
return ENOMEM;

ELF::Core::NotesEntryHeader null_entry {};
null_entry.type = ELF::Core::NotesEntryHeader::Type::Null;
notes_buffer.append(&null_entry, sizeof(null_entry));
if (!notes_buffer.try_append(&null_entry, sizeof(null_entry)))
return ENOMEM;

return notes_buffer;
}
Expand All @@ -324,7 +345,11 @@ KResult Coredump::write()
SpinlockLocker lock(m_process->address_space().get_lock());
ProcessPagingScope scope(m_process);

ByteBuffer notes_segment = create_notes_segment_data();
auto notes_segment_result = create_notes_segment_data();
if (notes_segment_result.is_error())
return notes_segment_result.error();

auto& notes_segment = notes_segment_result.value();

auto result = write_elf_header();
if (result.is_error())
Expand Down
10 changes: 5 additions & 5 deletions Kernel/Coredump.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ class Coredump {
[[nodiscard]] KResult write_regions();
[[nodiscard]] KResult write_notes_segment(ByteBuffer&);

ByteBuffer create_notes_segment_data() const;
ByteBuffer create_notes_process_data() const;
ByteBuffer create_notes_threads_data() const;
ByteBuffer create_notes_regions_data() const;
ByteBuffer create_notes_metadata_data() const;
KResultOr<ByteBuffer> create_notes_segment_data() const;
KResultOr<ByteBuffer> create_notes_process_data() const;
KResultOr<ByteBuffer> create_notes_threads_data() const;
KResultOr<ByteBuffer> create_notes_regions_data() const;
KResultOr<ByteBuffer> create_notes_metadata_data() const;

NonnullRefPtr<Process> m_process;
NonnullRefPtr<FileDescription> m_fd;
Expand Down
5 changes: 4 additions & 1 deletion Tests/LibTLS/TestTLSHandshake.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ TEST_CASE(test_TLS_hello_handshake)
loop.quit(1);
} else {
// print_buffer(data.value(), 16);
contents.append(data.value().data(), data.value().size());
if (!contents.try_append(data.value().data(), data.value().size())) {
FAIL("Allocation failure");
loop.quit(1);
}
}
};
tls->on_tls_finished = [&] {
Expand Down
3 changes: 2 additions & 1 deletion Userland/Applications/Debugger/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ static bool handle_disassemble_command(const String& command, void* first_instru
auto value = g_debug_session->peek(reinterpret_cast<u32*>(first_instruction) + i);
if (!value.has_value())
break;
code.append(&value, sizeof(u32));
if (!code.try_append(&value, sizeof(u32)))
break;
}

X86::SimpleInstructionStream stream(code.data(), code.size());
Expand Down
10 changes: 6 additions & 4 deletions Userland/Libraries/LibC/netdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ hostent* gethostbyaddr(const void* addr, socklen_t addr_size, int type)

struct servent* getservent()
{
//If the services file is not open, attempt to open it and return null if it fails.
// If the services file is not open, attempt to open it and return null if it fails.
if (!services_file) {
services_file = fopen(services_path, "r");

Expand Down Expand Up @@ -424,7 +424,7 @@ void endservent()
// false if failure occurs.
static bool fill_getserv_buffers(const char* line, ssize_t read)
{
//Splitting the line by tab delimiter and filling the servent buffers name, port, and protocol members.
// Splitting the line by tab delimiter and filling the servent buffers name, port, and protocol members.
String string_line = String(line, read);
string_line.replace(" ", "\t", true);
auto split_line = string_line.split('\t');
Expand Down Expand Up @@ -465,7 +465,8 @@ static bool fill_getserv_buffers(const char* line, ssize_t read)
break;
}
auto alias = split_line[i].to_byte_buffer();
alias.append("\0", sizeof(char));
if (!alias.try_append("\0", sizeof(char)))
return false;
__getserv_alias_list_buffer.append(move(alias));
}
}
Expand Down Expand Up @@ -635,7 +636,8 @@ static bool fill_getproto_buffers(const char* line, ssize_t read)
if (split_line[i].starts_with('#'))
break;
auto alias = split_line[i].to_byte_buffer();
alias.append("\0", sizeof(char));
if (!alias.try_append("\0", sizeof(char)))
return false;
__getproto_alias_list_buffer.append(move(alias));
}
}
Expand Down
5 changes: 4 additions & 1 deletion Userland/Libraries/LibCrypto/ASN1/PEM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ ByteBuffer decode_pem(ReadonlyBytes data)
break;
}
auto b64decoded = decode_base64(lexer.consume_line().trim_whitespace(TrimMode::Right));
decoded.append(b64decoded.data(), b64decoded.size());
if (!decoded.try_append(b64decoded.data(), b64decoded.size())) {
dbgln("Failed to decode PEM, likely OOM condition");
return {};
}
break;
}
case Ended:
Expand Down
5 changes: 4 additions & 1 deletion Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,10 @@ class EMSA_PSS : public Code<HashFunction> {
for (size_t counter = 0; counter < length / HashFunction::DigestSize - 1; ++counter) {
hash_fn.update(seed);
hash_fn.update((u8*)&counter, 4);
T.append(hash_fn.digest().data, HashFunction::DigestSize);
if (!T.try_append(hash_fn.digest().data, HashFunction::DigestSize)) {
dbgln("EMSA_PSS: MGF1 digest failed, not enough space");
return;
}
}
out.overwrite(0, T.data(), length);
}
Expand Down
3 changes: 2 additions & 1 deletion Userland/Libraries/LibGfx/BMPWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ ByteBuffer BMPWriter::dump(const RefPtr<Bitmap> bitmap, DibHeader dib_header)
}
}

buffer.append(pixel_data.data(), pixel_data.size());
if (!buffer.try_append(pixel_data.data(), pixel_data.size()))
dbgln("Failed to write {} bytes of pixel data to buffer", pixel_data.size());
return buffer;
}

Expand Down
4 changes: 1 addition & 3 deletions Userland/Libraries/LibGfx/PNGWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ PNGChunk::PNGChunk(String type)

void PNGChunk::store_type()
{
for (auto character : type()) {
m_data.append(&character, sizeof(character));
}
m_data.append(type().bytes());
}

void PNGChunk::store_data_length()
Expand Down
3 changes: 2 additions & 1 deletion Userland/Libraries/LibLine/Editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ void Editor::insert(const u32 cp)
StringBuilder builder;
builder.append(Utf32View(&cp, 1));
auto str = builder.build();
m_pending_chars.append(str.characters(), str.length());
if (!m_pending_chars.try_append(str.characters(), str.length()))
return;

readjust_anchored_styles(m_cursor, ModificationKind::Insertion);

Expand Down
6 changes: 4 additions & 2 deletions Userland/Libraries/LibPDF/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,10 @@ bool Parser::initialize_hint_tables()
auto total_size = primary_size + overflow_size;

possible_merged_stream_buffer = ByteBuffer::create_uninitialized(total_size);
possible_merged_stream_buffer.append(primary_hint_stream->bytes());
possible_merged_stream_buffer.append(overflow_hint_stream->bytes());
auto ok = possible_merged_stream_buffer.try_append(primary_hint_stream->bytes());
ok = ok && possible_merged_stream_buffer.try_append(overflow_hint_stream->bytes());
if (!ok)
return false;
hint_stream_bytes = possible_merged_stream_buffer.bytes();
} else {
hint_stream_bytes = primary_hint_stream->bytes();
Expand Down
3 changes: 2 additions & 1 deletion Userland/Libraries/LibSQL/Heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ bool Heap::write_block(u32 block, ByteBuffer& buffer)
VERIFY(buffer.size() <= BLOCKSIZE);
auto sz = buffer.size();
if (sz < BLOCKSIZE) {
buffer.resize(BLOCKSIZE);
if (!buffer.try_resize(BLOCKSIZE))
return false;
memset(buffer.offset_pointer((int)sz), 0, BLOCKSIZE - sz);
}
dbgln_if(SQL_DEBUG, "{:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x}",
Expand Down
5 changes: 4 additions & 1 deletion Userland/Libraries/LibTLS/HandshakeClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ bool TLSv12::compute_master_secret_from_pre_master_secret(size_t length)
return false;
}

m_context.master_key.resize(length);
if (!m_context.master_key.try_resize(length)) {
dbgln("Couldn't allocate enough space for the master key :(");
return false;
}

pseudorandom_function(
m_context.master_key,
Expand Down
Loading

0 comments on commit 3a9f00c

Please sign in to comment.