Skip to content

Commit

Permalink
Everywhere: Make ByteBuffer::{create_*,copy}() OOM-safe
Browse files Browse the repository at this point in the history
  • Loading branch information
alimpfard authored and awesomekling committed Sep 5, 2021
1 parent 3a9f00c commit 97e97bc
Show file tree
Hide file tree
Showing 105 changed files with 629 additions and 290 deletions.
3 changes: 2 additions & 1 deletion AK/Base64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ ByteBuffer decode_base64(const StringView& input)
output.append(out2);
}

return ByteBuffer::copy(output.data(), output.size());
// FIXME: Handle OOM failure.
return ByteBuffer::copy(output).release_value();
}

String encode_base64(ReadonlyBytes input)
Expand Down
33 changes: 20 additions & 13 deletions AK/ByteBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,31 +62,35 @@ class ByteBuffer {
return *this;
}

[[nodiscard]] static ByteBuffer create_uninitialized(size_t size)
[[nodiscard]] static Optional<ByteBuffer> create_uninitialized(size_t size)
{
auto buffer = ByteBuffer();
auto ok = buffer.try_resize(size);
VERIFY(ok);
return buffer;
if (!buffer.try_resize(size))
return {};
return { move(buffer) };
}

[[nodiscard]] static ByteBuffer create_zeroed(size_t size)
[[nodiscard]] static Optional<ByteBuffer> create_zeroed(size_t size)
{
auto buffer = create_uninitialized(size);
auto buffer_result = create_uninitialized(size);
if (!buffer_result.has_value())
return {};

auto& buffer = buffer_result.value();
buffer.zero_fill();
VERIFY(size == 0 || (buffer[0] == 0 && buffer[size - 1] == 0));
return buffer;
return buffer_result;
}

[[nodiscard]] static ByteBuffer copy(void const* data, size_t size)
[[nodiscard]] static Optional<ByteBuffer> copy(void const* data, size_t size)
{
auto buffer = create_uninitialized(size);
if (size != 0)
__builtin_memcpy(buffer.data(), data, size);
if (buffer.has_value() && size != 0)
__builtin_memcpy(buffer->data(), data, size);
return buffer;
}

[[nodiscard]] static ByteBuffer copy(ReadonlyBytes bytes)
[[nodiscard]] static Optional<ByteBuffer> copy(ReadonlyBytes bytes)
{
return copy(bytes.data(), bytes.size());
}
Expand Down Expand Up @@ -133,12 +137,13 @@ class ByteBuffer {
[[nodiscard]] void* end_pointer() { return data() + m_size; }
[[nodiscard]] void const* end_pointer() const { return data() + m_size; }

// FIXME: Make this function handle failures too.
[[nodiscard]] ByteBuffer slice(size_t offset, size_t size) const
{
// I cannot hand you a slice I don't have
VERIFY(offset + size <= this->size());

return copy(offset_pointer(offset), size);
return copy(offset_pointer(offset), size).release_value();
}

void clear()
Expand Down Expand Up @@ -237,8 +242,10 @@ class ByteBuffer {
if (!other.m_inline) {
m_outline_buffer = other.m_outline_buffer;
m_outline_capacity = other.m_outline_capacity;
} else
} else {
VERIFY(other.m_size <= inline_capacity);
__builtin_memcpy(m_inline_buffer, other.m_inline_buffer, other.m_size);
}
other.m_size = 0;
other.m_inline = true;
}
Expand Down
8 changes: 6 additions & 2 deletions AK/Hex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ Optional<ByteBuffer> decode_hex(const StringView& input)
if ((input.length() % 2) != 0)
return {};

auto output = ByteBuffer::create_zeroed(input.length() / 2);
auto output_result = ByteBuffer::create_zeroed(input.length() / 2);
if (!output_result.has_value())
return {};

auto& output = output_result.value();

for (size_t i = 0; i < input.length() / 2; ++i) {
const auto c1 = decode_hex_digit(input[i * 2]);
Expand All @@ -34,7 +38,7 @@ Optional<ByteBuffer> decode_hex(const StringView& input)
output[i] = (c1 << 4) + c2;
}

return output;
return output_result;
}

String encode_hex(const ReadonlyBytes input)
Expand Down
5 changes: 3 additions & 2 deletions AK/MemoryStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class DuplexMemoryStream final : public DuplexStream {
size_t nwritten = 0;
while (bytes.size() - nwritten > 0) {
if ((m_write_offset + nwritten) % chunk_size == 0)
m_chunks.append(ByteBuffer::create_uninitialized(chunk_size));
m_chunks.append(ByteBuffer::create_uninitialized(chunk_size).release_value()); // FIXME: Handle possible OOM situation.

nwritten += bytes.slice(nwritten).copy_trimmed_to(m_chunks.last().bytes().slice((m_write_offset + nwritten) % chunk_size));
}
Expand All @@ -241,7 +241,8 @@ class DuplexMemoryStream final : public DuplexStream {

ByteBuffer copy_into_contiguous_buffer() const
{
auto buffer = ByteBuffer::create_uninitialized(size());
// FIXME: Handle possible OOM situation.
auto buffer = ByteBuffer::create_uninitialized(size()).release_value();

const auto nread = read_without_consuming(buffer);
VERIFY(nread == buffer.size());
Expand Down
3 changes: 2 additions & 1 deletion AK/String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ ByteBuffer String::to_byte_buffer() const
{
if (!m_impl)
return {};
return ByteBuffer::copy(reinterpret_cast<const u8*>(characters()), length());
// FIXME: Handle OOM failure.
return ByteBuffer::copy(bytes()).release_value();
}

template<typename T>
Expand Down
3 changes: 2 additions & 1 deletion AK/StringBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ void StringBuilder::appendvf(char const* fmt, va_list ap)

ByteBuffer StringBuilder::to_byte_buffer() const
{
return ByteBuffer::copy(data(), length());
// FIXME: Handle OOM failure.
return ByteBuffer::copy(data(), length()).release_value();
}

String StringBuilder::to_string() const
Expand Down
33 changes: 26 additions & 7 deletions Kernel/FileSystem/Ext2FileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ KResult Ext2FSInode::write_indirect_block(BlockBasedFileSystem::BlockIndex block
const auto entries_per_block = EXT2_ADDR_PER_BLOCK(&fs().super_block());
VERIFY(blocks_indices.size() <= entries_per_block);

auto block_contents = ByteBuffer::create_uninitialized(fs().block_size());
auto block_contents_result = ByteBuffer::create_uninitialized(fs().block_size());
if (!block_contents_result.has_value())
return ENOMEM;
auto block_contents = block_contents_result.release_value();
OutputMemoryStream stream { block_contents };
auto buffer = UserOrKernelBuffer::for_kernel_buffer(stream.data());

Expand All @@ -235,7 +238,10 @@ KResult Ext2FSInode::grow_doubly_indirect_block(BlockBasedFileSystem::BlockIndex
VERIFY(blocks_indices.size() > old_blocks_length);
VERIFY(blocks_indices.size() <= entries_per_doubly_indirect_block);

auto block_contents = ByteBuffer::create_uninitialized(fs().block_size());
auto block_contents_result = ByteBuffer::create_uninitialized(fs().block_size());
if (!block_contents_result.has_value())
return ENOMEM;
auto block_contents = block_contents_result.release_value();
auto* block_as_pointers = (unsigned*)block_contents.data();
OutputMemoryStream stream { block_contents };
auto buffer = UserOrKernelBuffer::for_kernel_buffer(stream.data());
Expand Down Expand Up @@ -275,7 +281,10 @@ KResult Ext2FSInode::shrink_doubly_indirect_block(BlockBasedFileSystem::BlockInd
VERIFY(old_blocks_length >= new_blocks_length);
VERIFY(new_blocks_length <= entries_per_doubly_indirect_block);

auto block_contents = ByteBuffer::create_uninitialized(fs().block_size());
auto block_contents_result = ByteBuffer::create_uninitialized(fs().block_size());
if (!block_contents_result.has_value())
return ENOMEM;
auto block_contents = block_contents_result.release_value();
auto* block_as_pointers = (unsigned*)block_contents.data();
auto buffer = UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<u8*>(block_as_pointers));
TRY(fs().read_block(block, &buffer, fs().block_size()));
Expand Down Expand Up @@ -308,7 +317,10 @@ KResult Ext2FSInode::grow_triply_indirect_block(BlockBasedFileSystem::BlockIndex
VERIFY(blocks_indices.size() > old_blocks_length);
VERIFY(blocks_indices.size() <= entries_per_triply_indirect_block);

auto block_contents = ByteBuffer::create_uninitialized(fs().block_size());
auto block_contents_result = ByteBuffer::create_uninitialized(fs().block_size());
if (!block_contents_result.has_value())
return ENOMEM;
auto block_contents = block_contents_result.release_value();
auto* block_as_pointers = (unsigned*)block_contents.data();
OutputMemoryStream stream { block_contents };
auto buffer = UserOrKernelBuffer::for_kernel_buffer(stream.data());
Expand Down Expand Up @@ -351,7 +363,10 @@ KResult Ext2FSInode::shrink_triply_indirect_block(BlockBasedFileSystem::BlockInd
VERIFY(old_blocks_length >= new_blocks_length);
VERIFY(new_blocks_length <= entries_per_triply_indirect_block);

auto block_contents = ByteBuffer::create_uninitialized(fs().block_size());
auto block_contents_result = ByteBuffer::create_uninitialized(fs().block_size());
if (!block_contents_result.has_value())
return ENOMEM;
auto block_contents = block_contents_result.release_value();
auto* block_as_pointers = (unsigned*)block_contents.data();
auto buffer = UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<u8*>(block_as_pointers));
TRY(fs().read_block(block, &buffer, fs().block_size()));
Expand Down Expand Up @@ -583,7 +598,8 @@ Vector<Ext2FS::BlockIndex> Ext2FSInode::compute_block_list_impl_internal(const e
if (!count)
return;
size_t read_size = count * sizeof(u32);
auto array_storage = ByteBuffer::create_uninitialized(read_size);
// FIXME: Handle possible OOM situation.
auto array_storage = ByteBuffer::create_uninitialized(read_size).release_value();
auto* array = (u32*)array_storage.data();
auto buffer = UserOrKernelBuffer::for_kernel_buffer((u8*)array);
if (auto result = fs().read_block(array_block_index, &buffer, read_size, 0); result.is_error()) {
Expand Down Expand Up @@ -1109,7 +1125,10 @@ KResult Ext2FSInode::write_directory(Vector<Ext2FSDirectoryEntry>& entries)

dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_directory(): New directory contents to write (size {}):", identifier(), directory_size);

auto directory_data = ByteBuffer::create_uninitialized(directory_size);
auto directory_data_result = ByteBuffer::create_uninitialized(directory_size);
if (!directory_data_result.has_value())
return ENOMEM;
auto directory_data = directory_data_result.release_value();
OutputMemoryStream stream { directory_data };

for (auto& entry : entries) {
Expand Down
31 changes: 22 additions & 9 deletions Kernel/Net/NE2000NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ UNMAP_AFTER_INIT int NE2000NetworkAdapter::ram_test()
out8(REG_WR_REMOTEBYTECOUNT1, 0x00);
out8(REG_WR_RECEIVECONFIGURATION, BIT_RECEIVECONFIGURATION_MON);
out8(REG_RW_COMMAND, BIT_COMMAND_DMA_ABORT | BIT_COMMAND_START);
auto buffer = ByteBuffer::create_uninitialized(NE2K_RAM_SIZE);
Array<u8, NE2K_RAM_SIZE> buffer;

const u8 patterns[3] = { 0x5a, 0xff, 0x00 };
for (int i = 0; i < 3; ++i) {
Expand Down Expand Up @@ -412,22 +412,35 @@ void NE2000NetworkAdapter::receive()
dbgln_if(NE2000_DEBUG, "NE2000NetworkAdapter: Packet received {} length={}", (packet_ok ? "intact" : "damaged"), header.length);

if (packet_ok) {
auto packet = NetworkByteBuffer::create_uninitialized(sizeof(received_packet_header) + header.length);
int bytes_left = packet.size();
size_t bytes_in_packet = sizeof(received_packet_header) + header.length;

auto packet_result = NetworkByteBuffer::create_uninitialized(bytes_in_packet);
u8 drop_buffer[NE2K_PAGE_SIZE];
Bytes buffer { drop_buffer, array_size(drop_buffer) };
bool will_drop { false };
if (!packet_result.has_value()) {
dbgln("NE2000NetworkAdapter: Not enough memory for packet with length = {}, dropping.", header.length);
will_drop = true;
} else {
buffer = packet_result->bytes();
}

int current_offset = 0;
int ring_offset = header_address;

while (bytes_left > 0) {
int copy_size = min(bytes_left, NE2K_PAGE_SIZE);
rdma_read(ring_offset, packet.span().slice(current_offset, copy_size));
current_offset += copy_size;
while (bytes_in_packet > 0) {
int copy_size = min(bytes_in_packet, NE2K_PAGE_SIZE);
rdma_read(ring_offset, buffer.slice(current_offset, copy_size));
if (!will_drop)
current_offset += copy_size;
ring_offset += copy_size;
bytes_left -= copy_size;
bytes_in_packet -= copy_size;
if (ring_offset == NE2K_RAM_RECV_END)
ring_offset = NE2K_RAM_RECV_BEGIN;
}

did_receive(packet.span().slice(sizeof(received_packet_header)));
if (!will_drop)
did_receive(buffer.slice(sizeof(received_packet_header)));
}

if (header.next_packet_page == (NE2K_RAM_RECV_BEGIN >> 8))
Expand Down
8 changes: 6 additions & 2 deletions Kernel/Net/NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ void NetworkAdapter::send_packet(ReadonlyBytes packet)
void NetworkAdapter::send(const MACAddress& destination, const ARPPacket& packet)
{
size_t size_in_bytes = sizeof(EthernetFrameHeader) + sizeof(ARPPacket);
auto buffer = NetworkByteBuffer::create_zeroed(size_in_bytes);
auto* eth = (EthernetFrameHeader*)buffer.data();
auto buffer_result = NetworkByteBuffer::create_zeroed(size_in_bytes);
if (!buffer_result.has_value()) {
dbgln("Dropping ARP packet targeted at {} as there is not enough memory to buffer it", packet.target_hardware_address().to_string());
return;
}
auto* eth = (EthernetFrameHeader*)buffer_result->data();
eth->set_source(mac_address());
eth->set_destination(destination);
eth->set_ether_type(EtherType::ARP);
Expand Down
16 changes: 13 additions & 3 deletions Kernel/Random.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ class FortunaPRNG {
using HashType = HashT;
using DigestType = typename HashT::DigestType;

// FIXME: Do something other than VERIFY()'ing inside Optional in case of OOM.
FortunaPRNG()
: m_counter(ByteBuffer::create_zeroed(BlockType::block_size()))
: m_counter(ByteBuffer::create_zeroed(BlockType::block_size()).release_value())
{
}

Expand Down Expand Up @@ -95,8 +96,17 @@ class FortunaPRNG {
}
}
DigestType digest = new_key.digest();
m_key = ByteBuffer::copy(digest.immutable_data(),
digest.data_length());
if (m_key.size() == digest.data_length()) {
// Avoid reallocating, just overwrite the key.
m_key.overwrite(0, digest.immutable_data(), digest.data_length());
} else {
auto buffer_result = ByteBuffer::copy(digest.immutable_data(), digest.data_length());
// If there's no memory left to copy this into, bail out.
if (!buffer_result.has_value())
return;

m_key = buffer_result.release_value();
}

m_reseed_number++;
m_p0_len = 0;
Expand Down
5 changes: 3 additions & 2 deletions Kernel/Storage/IDEChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,9 @@ UNMAP_AFTER_INIT void IDEChannel::detect_disks()
}
}

ByteBuffer wbuf = ByteBuffer::create_uninitialized(512);
ByteBuffer bbuf = ByteBuffer::create_uninitialized(512);
// FIXME: Handle possible OOM situation here.
ByteBuffer wbuf = ByteBuffer::create_uninitialized(512).release_value();
ByteBuffer bbuf = ByteBuffer::create_uninitialized(512).release_value();
u8* b = bbuf.data();
u16* w = (u16*)wbuf.data();

Expand Down
10 changes: 8 additions & 2 deletions Kernel/Storage/Partition/GUIDPartitionTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ Result<NonnullOwnPtr<GUIDPartitionTable>, PartitionTable::Error> GUIDPartitionTa
GUIDPartitionTable::GUIDPartitionTable(const StorageDevice& device)
: MBRPartitionTable(device)
{
m_cached_header = ByteBuffer::create_zeroed(m_device->block_size());
// FIXME: Handle OOM failure here.
m_cached_header = ByteBuffer::create_zeroed(m_device->block_size()).release_value();
VERIFY(partitions_count() == 0);
if (!initialize())
m_valid = false;
Expand Down Expand Up @@ -87,7 +88,12 @@ bool GUIDPartitionTable::initialize()
return false;
}

auto entries_buffer = ByteBuffer::create_zeroed(m_device->block_size());
auto entries_buffer_result = ByteBuffer::create_zeroed(m_device->block_size());
if (!entries_buffer_result.has_value()) {
dbgln("GUIPartitionTable: not enough memory for entries buffer");
return false;
}
auto entries_buffer = entries_buffer_result.release_value();
auto raw_entries_buffer = UserOrKernelBuffer::for_kernel_buffer(entries_buffer.data());
size_t raw_byte_index = header().partition_array_start_lba * m_device->block_size();
for (size_t entry_index = 0; entry_index < header().entries_count; entry_index++) {
Expand Down
4 changes: 2 additions & 2 deletions Kernel/Storage/Partition/MBRPartitionTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ bool MBRPartitionTable::read_boot_record()
MBRPartitionTable::MBRPartitionTable(const StorageDevice& device, u32 start_lba)
: PartitionTable(device)
, m_start_lba(start_lba)
, m_cached_header(ByteBuffer::create_zeroed(m_device->block_size()))
, m_cached_header(ByteBuffer::create_zeroed(m_device->block_size()).release_value()) // FIXME: Do something sensible if this fails because of OOM.
{
if (!read_boot_record() || !initialize())
return;
Expand All @@ -68,7 +68,7 @@ MBRPartitionTable::MBRPartitionTable(const StorageDevice& device, u32 start_lba)
MBRPartitionTable::MBRPartitionTable(const StorageDevice& device)
: PartitionTable(device)
, m_start_lba(0)
, m_cached_header(ByteBuffer::create_zeroed(m_device->block_size()))
, m_cached_header(ByteBuffer::create_zeroed(m_device->block_size()).release_value()) // FIXME: Do something sensible if this fails because of OOM.
{
if (!read_boot_record() || contains_ebr() || is_protective_mbr() || !initialize())
return;
Expand Down
Loading

0 comments on commit 97e97bc

Please sign in to comment.