MVP Loader implementation #458

roblabla · 2019-08-27T09:46:03Z

MVP loader. Things to do before merging this PR:

Implement and use unmap_process_memory.
Various unimportant TODOs.
Fix clippy warnings.

todo · 2019-08-27T09:46:06Z

sagiegurari/shell2batch#9

Lines 224 to 234 in 2ba154e

    
           # TODO: https://github.com/sagiegurari/shell2batch/issues/9 
        
           touch external/filesystem/disk_template/bin/0200000000000000/flags/boot.flag 
        
           cargo run --manifest-path disk-initializer/Cargo.toml -- DISK.img 52428800 external/filesystem/disk_template/ 
        
           ''' 
        
           ] 
        
           [tasks.qemu] 
        
           description = "Runs the bootable ISO in qemu." 
        
           dependencies = ["iso-release", "disk"] 
        
           command = "qemu-system-i386" 
        
           args = [

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

todo · 2019-08-27T09:46:09Z

Write our threat models. Will require careful planning.

SunriseOS/docs/SECURITY_ARCHITECTURE.md

Lines 155 to 161 in 2ba154e

    
           TODO: Write our threat models. Will require careful planning. 
        
           [Boot Configuration Table (BCT)]: https://switchbrew.org/wiki/BCT 
        
           [Package1]: https://switchbrew.org/wiki/Package1 
        
           [switchbrew wiki]: https://switchbrew.org/ 
        
           [NPDM]: http://switchbrew.org/index.php?title=NPDM

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

todo · 2019-08-27T09:46:11Z

ASLR

We should generate a random aslr base.

SunriseOS/kernel/src/main.rs

Lines 153 to 163 in 2ba154e

    
           // TODO: ASLR 
        
           // BODY: We should generate a random aslr base. 
        
           let aslr_base = 0x400000; 
        
           let procinfo = ProcInfo { 
        
               name: name, 
        
               process_category: ProcessCategory::KernelBuiltin, 
        
               // TODO: Put TitleId somewhere in the ELF or args or something? 
        
               title_id: 0, 
        
               code_addr: aslr_base as _, 
        
               // We don't need this, the kernel loader allocates multiple code

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

todo · 2019-08-27T09:46:18Z

Ask the weird animal if this will unleash the kraken.

SunriseOS/kernel/src/paging/arch/i386/lands.rs

Lines 61 to 63 in 2ba154e

    
           // TODO: Ask the weird animal if this will unleash the kraken. 
        
           //const_assert!(UserLand::START.0   % (ENTRY_COUNT * PAGE_SIZE) == 0); 
        
           const_assert!(RecursiveTablesLand::START.0 % (ENTRY_COUNT * PAGE_SIZE) == 0);

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

todo · 2019-08-27T09:46:39Z

Create memory region reservations

Memory region reservations is sort of insane in HOS/NX - especially for 32-bit. I'll figure it out later.

SunriseOS/kernel/src/syscalls.rs

Lines 948 to 958 in 2ba154e

    
               // TODO: Create memory region reservations 
        
               // BODY: Memory region reservations is sort of insane in HOS/NX - especially 
        
               // BODY: for 32-bit. I'll figure it out later. 
        
               newproc.pmemory.lock().create_regular_mapping(VirtualAddress(procinfo.code_addr as usize), procinfo.code_num_pages as usize * PAGE_SIZE, MemoryType::CodeStatic, MappingAccessRights::k_r())?; 
        
               let curproc = scheduler::get_current_process(); 
        
               let hnd = curproc.phandles.lock().add_handle(Arc::new(Handle::Process(newproc))); 
        
               Ok(hnd as _) 
        
           }

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

todo · 2019-08-27T09:46:41Z

Use svcGetInfo to get the address space in find_free_address

We should use svcGetInfo to get the address space in `find_free_address`. This is extremely important as the low addresses (from 0 to ADDRESS_SPACE_MIN) are not usable, but are marked as available by QueryMemory.
Here's a sample implementation for when we'll have GetInfo impl'd.
```rust lazy_static! { static ref ADDRESS_SPACE: (usize, usize) = { let addr_space_base = syscalls::get_info(Process::current(), 12, 0).unwrap(); let addr_space_size = syscalls::get_info(Process::current(), 13, 0).unwrap(); (addr_space_base, addr_space_base + addr_space_size) }; } ```

SunriseOS/libuser/src/mem.rs

Lines 25 to 35 in 2ba154e

    
           // TODO: Use svcGetInfo to get the address space in find_free_address 
        
           // BODY: We should use svcGetInfo to get the address space in 
        
           // BODY: `find_free_address`. This is extremely important as the low 
        
           // BODY: addresses (from 0 to ADDRESS_SPACE_MIN) are not usable, but are 
        
           // BODY: marked as available by QueryMemory. 
        
           // BODY: 
        
           // BODY: Here's a sample implementation for when we'll have GetInfo impl'd. 
        
           // BODY: 
        
           // BODY: ```rust 
        
           // BODY: lazy_static! { 
        
           // BODY:     static ref ADDRESS_SPACE: (usize, usize) = {

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

marysaka

lgtm

kernel/src/paging/arch/i386/lands.rs

Orycterope · 2019-09-09T12:37:53Z

libkern/src/process.rs

+    Sysmodule,
+    /// Pool of memory usable by nvidia's driver.
+    ///
+    /// <Insert nvidia meme here>


libkern/src/lib.rs

Orycterope · 2019-09-09T14:41:53Z

kernel/src/syscalls.rs

+        if meminfo.address() < addr {
+            dstmem.map_partial_shared_mapping(frames.clone(), meminfo.address(), meminfo.phys_offset(), addr - meminfo.address(), meminfo.state().ty(), meminfo.flags()).expect("Can't fail");
+        }
+        if meminfo.address() + meminfo.length() > addr + size && meminfo.length() > PAGE_SIZE {


Ok, you're checking if there's an after part, and remapping it as is. But why the meminfo.length() > PAGE_SIZE condition ?

loader/src/elf_loader.rs

loader/src/main.rs

Orycterope · 2019-09-10T11:18:25Z

loader/src/main.rs

+
+    if size > MAX_ELF_SIZE {
+        error!("Why is titleid {:016x} so ridiculously huge? It's {} bytes.
+        Like, seriously, stop with the gifs!", titleid, size);


Orycterope · 2019-09-10T12:41:50Z

kernel/src/syscalls.rs

+    // BODY: a vector of frames) and comparing them.
+    // BODY:
+    // BODY: We could do something similar by iterating over the Mappings and
+    // BODY: checking if their Frames + PhysOffset are equals.


This syscall is really shitty.

I don't get why it didn't simply create a handle of some kind, and you could simply unmap the handle as a whole.

🤷‍♂ I don't find it all that bad. Just maybe a bit too flexible.

I do hate that closing the handle doesn't automatically unmap the frames though.

todo · 2019-09-11T16:35:29Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in 9e93f28

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `9e93f28` in #458. cc @roblabla.

todo · 2019-09-11T16:39:47Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in e1d6e43

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `e1d6e43` in #458. cc @roblabla.

todo · 2019-09-12T09:07:49Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in 2d7cdb3

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `2d7cdb3` in #458. cc @roblabla.

Orycterope · 2019-09-16T21:16:27Z

kernel/src/process.rs

+            ProcessState::CreatedAttached => ProcessState::StartedAttached,
+            _ => unreachable!()
+        };
+        this.state.store(newstate, Ordering::SeqCst);


textbook read-modify-write race condition.

You need to compare exchange the states, to arbitrate who's the winner of the race, and gets to actually start it.

This is exactly why I believe we should be mutexing this stuff :P

- Process::new now takes a ProcInfo, and derives all necessary information from this. - Specify aslr base in a single location. - Make UserLand address space start at 0x00200000 - Implement create_process syscall, as accurately as currently possible.

MapProcessMemory will allow loader to map the remote code created in CreateProcess in its own address space in order to load the ELf into it.

This function mimicks nintendo's KMemoryManager::CheckRange. It goes over a memory region and checks that it is in the expected state (both in terms of MemoryState, MemoryPermissions and MemoryAttributes), and that it is homogenous (that is, all submappings have the same state and perms). This is used to significantly simplify set_process_memory_permission and map_process_memory.

Used to unmap memory mapped with MapProcessMemory. Loader now unmaps the process memory it previously mapped.

Stop unwrapping left and right and return actual errors. This means we should no longer be crashing if we send an invalid binary to loader. Instead, loader will log an error and hum along.

Avoids starting a process twice. Will in the future allow tracking crashes, and allow waiting on a process from userspace to see when it changes state.

todo · 2019-09-17T14:07:47Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in 3b26b34

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `3b26b34` in #458. cc @roblabla.

todo · 2019-09-17T15:35:48Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in 06f20e2

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `06f20e2` in #458. cc @roblabla.

todo · 2019-09-17T17:25:54Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 952 to 962 in 5b48fde

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `5b48fde` in #458. cc @roblabla.

todo · 2019-09-18T11:22:11Z

UnmapProcessMemory: Verify that the src page list == dst page list.

In UnmapProcessMemory, we don't ensure that src_address is correct, that is, we don't check that the frames in the dst match the frame in the src. HOS/NX does this by building a PageList (essentially a vector of frames) and comparing them.
We could do something similar by iterating over the Mappings and checking if their Frames + PhysOffset are equals.

SunriseOS/kernel/src/syscalls.rs

Lines 954 to 964 in 27ab3e4

    
           // TODO: UnmapProcessMemory: Verify that the src page list == dst page list. 
        
           // BODY: In UnmapProcessMemory, we don't ensure that src_address is correct, 
        
           // BODY: that is, we don't check that the frames in the dst match the frame 
        
           // BODY: in the src. HOS/NX does this by building a PageList (essentially 
        
           // BODY: a vector of frames) and comparing them. 
        
           // BODY: 
        
           // BODY: We could do something similar by iterating over the Mappings and 
        
           // BODY: checking if their Frames + PhysOffset are equals. 
        
           // Unmap. 
        
           while size != 0 {

This comment was generated by todo based on a `TODO` comment in `27ab3e4` in #458. cc @roblabla.

roblabla · 2019-09-18T11:25:36Z

seriously, todobot, you're drunk.

Hopefully this time travis passes and we'll get this merged.

This comment has been minimized.

Sign in to view

roblabla force-pushed the loader branch from 2ba154e to eb9dda7 Compare August 28, 2019 14:39

This comment has been minimized.

Sign in to view

marysaka requested review from Orycterope and marysaka September 4, 2019 21:44

marysaka approved these changes Sep 4, 2019

View reviewed changes

Orycterope requested changes Sep 10, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

roblabla force-pushed the loader branch from e1d6e43 to 2d7cdb3 Compare September 12, 2019 09:07

Orycterope approved these changes Sep 16, 2019

View reviewed changes

roblabla added 3 commits September 17, 2019 13:51

Kernel: Implement svcCreateProcess

5ae836e

- Process::new now takes a ProcInfo, and derives all necessary information from this. - Specify aslr base in a single location. - Make UserLand address space start at 0x00200000 - Implement create_process syscall, as accurately as currently possible.

Add security architecture docs

95c16e7

Kernel: Implement MapProcessMemory

0d92b83

MapProcessMemory will allow loader to map the remote code created in CreateProcess in its own address space in order to load the ELf into it.

roblabla added 14 commits September 17, 2019 13:51

fixup! Kernel: Add KIP Header to every KIP.

6f1cb7e

Use plain to read KipHeader

64d4aff

fixup! Kernel: Implement svcCreateProcess

e5fbc0c

Kernel: Implement UnmapProcessMemory

3d32fbf

Used to unmap memory mapped with MapProcessMemory. Loader now unmaps the process memory it previously mapped.

Keep correct order in the RegistersOnStack struct

08f54ae

Werid animal says this is fine

5b6eaff

Document bit twiddling magic in MemoryPermissions::check

fc77c74

Loader: Proper error handling.

08c7bb3

Stop unwrapping left and right and return actual errors. This means we should no longer be crashing if we send an invalid binary to loader. Instead, loader will log an error and hum along.

Use the atomic crate to implement AtomicThreadStruct

c757a90

Kernel: Add ProcessStruct::state

02a3e82

Avoids starting a process twice. Will in the future allow tracking crashes, and allow waiting on a process from userspace to see when it changes state.

Kernel: remove extra useless check in split mapping check

e6008ff

Kernel: Proper CAS when starting process.

5fdf247

Travis: Properly test the disk task

b0ad410

roblabla force-pushed the loader branch from 2d7cdb3 to 3b26b34 Compare September 17, 2019 14:07

Fix clippy

06f20e2

roblabla force-pushed the loader branch from 3b26b34 to 06f20e2 Compare September 17, 2019 15:35

Fix clippy again

5b48fde

Fix doc links

27ab3e4

roblabla merged commit 06c56ab into sunriseos:master Sep 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MVP Loader implementation #458

MVP Loader implementation #458

roblabla commented Aug 27, 2019 •

edited

Loading

todo bot commented Aug 27, 2019

todo bot commented Aug 27, 2019

todo bot commented Aug 27, 2019

This comment has been minimized.

todo bot commented Aug 27, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

todo bot commented Aug 27, 2019

todo bot commented Aug 27, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

marysaka left a comment

Orycterope Sep 9, 2019

Orycterope Sep 9, 2019

roblabla Sep 10, 2019 •

edited

Loading

Orycterope Sep 10, 2019

Orycterope Sep 10, 2019

roblabla Sep 10, 2019

This comment has been minimized.

todo bot commented Sep 11, 2019

todo bot commented Sep 11, 2019

todo bot commented Sep 12, 2019

Orycterope Sep 16, 2019

roblabla Sep 17, 2019

todo bot commented Sep 17, 2019

todo bot commented Sep 17, 2019

todo bot commented Sep 17, 2019

todo bot commented Sep 18, 2019

roblabla commented Sep 18, 2019

MVP Loader implementation #458

MVP Loader implementation #458

Conversation

roblabla commented Aug 27, 2019 • edited Loading

todo bot commented Aug 27, 2019

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

todo bot commented Aug 27, 2019

Write our threat models. Will require careful planning.

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

todo bot commented Aug 27, 2019

ASLR

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

This comment has been minimized.

todo bot commented Aug 27, 2019

Ask the weird animal if this will unleash the kraken.

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

todo bot commented Aug 27, 2019

Create memory region reservations

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

todo bot commented Aug 27, 2019

Use svcGetInfo to get the address space in find_free_address

This comment was generated by todo based on a TODO comment in 2ba154e in #458. cc @roblabla.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

marysaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roblabla Sep 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

todo bot commented Sep 11, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 9e93f28 in #458. cc @roblabla.

todo bot commented Sep 11, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in e1d6e43 in #458. cc @roblabla.

todo bot commented Sep 12, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 2d7cdb3 in #458. cc @roblabla.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

todo bot commented Sep 17, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 3b26b34 in #458. cc @roblabla.

todo bot commented Sep 17, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 06f20e2 in #458. cc @roblabla.

todo bot commented Sep 17, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 5b48fde in #458. cc @roblabla.

todo bot commented Sep 18, 2019

UnmapProcessMemory: Verify that the src page list == dst page list.

This comment was generated by todo based on a TODO comment in 27ab3e4 in #458. cc @roblabla.

roblabla commented Sep 18, 2019

roblabla commented Aug 27, 2019 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2ba154e` in #458. cc @roblabla.

roblabla Sep 10, 2019 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `9e93f28` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `e1d6e43` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `2d7cdb3` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `3b26b34` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `06f20e2` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `5b48fde` in #458. cc @roblabla.

This comment was generated by todo based on a `TODO` comment in `27ab3e4` in #458. cc @roblabla.