Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

X/C Buffers #229

Merged
merged 10 commits into from
Mar 19, 2019
Merged

X/C Buffers #229

merged 10 commits into from
Mar 19, 2019

Conversation

roblabla
Copy link
Member

@roblabla roblabla commented Mar 16, 2019

Support for IPC buffers.

@roblabla roblabla force-pushed the buffers branch 3 times, most recently from c4048b2 to 1c9da0e Compare March 16, 2019 21:24
@todo
Copy link

todo bot commented Mar 16, 2019

handle NULL addr case -> no need to map anything, just create an equivalent null entry.

https://github.com/roblabla42/KFS/blob/1c9da0ee3db7f9dcc5c10a9ba9b20931eb347b90/kernel/src/ipc/session.rs#L243-L251


This comment was generated by todo based on a TODO comment in 1c9da0e in #229. cc @roblabla.

@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@todo
Copy link

todo bot commented Mar 16, 2019

Check if from/to are > usize::max_value()

https://github.com/roblabla42/KFS/blob/1c9da0ee3db7f9dcc5c10a9ba9b20931eb347b90/kernel/src/ipc/session.rs#L547-L557


This comment was generated by todo based on a TODO comment in 1c9da0e in #229. cc @roblabla.

@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@sunriseos sunriseos deleted a comment from todo bot Mar 16, 2019
@todo
Copy link

todo bot commented Mar 17, 2019

Properly split this up.

https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/kernel/src/error.rs#L98-L105


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@todo
Copy link

todo bot commented Mar 17, 2019

handle NULL addr case -> no need to map anything, just create an equivalent null entry.

https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/kernel/src/ipc/session.rs#L243-L251


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@todo
Copy link

todo bot commented Mar 17, 2019

IPC Type-X: Prevent multiple writes to a C-buffer?

Do I need to prevent multiple writes to the same buffer ID? In theory, I could use coff as a bitmap of used buffers, and prevent reuse this way, but I'm unsure of how the nintendo switch behaves here.


https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/kernel/src/ipc/session.rs#L539-L549


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@todo
Copy link

todo bot commented Mar 17, 2019

Check if from/to are > usize::max_value()

https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/kernel/src/ipc/session.rs#L549-L559


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@todo
Copy link

todo bot commented Mar 17, 2019

Move pack to a non-generic function

Right now the pack and unpack functions are duplicated for every instanciation of Message. This probably has a huge penalty on codesize. We should make a function taking everything as slices instead


https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/libuser/src/ipc/mod.rs#L535-L545


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@todo
Copy link

todo bot commented Mar 17, 2019

Pointer Buf should take its size as a generic parameter.

The Pointer Buffer size should be configurable by the sysmodule. We'll wait for const generics to do it however, as otherwise we'd have to bend over backwards with typenum.


https://github.com/roblabla42/KFS/blob/1f47988ffdf455f5e35fb767446da72dfe39b17a/libuser/src/ipc/server.rs#L159-L166


This comment was generated by todo based on a TODO comment in 1f47988 in #229. cc @roblabla.

@roblabla roblabla marked this pull request as ready for review March 17, 2019 15:36
@todo
Copy link

todo bot commented Mar 17, 2019

buf_map: Check that from_mem has the right permissions

buf_map currently remaps without checking the permissions. This could allow a user to read non-user-accessible memory, or write to non-writable memory.
Whatever mechanism we setup for UserSpacePtr, we should probably reuse it here.


https://github.com/roblabla42/KFS/blob/54a1fd5b5203a2573c86d0ae2c868e750c53deb2/kernel/src/ipc/session.rs#L238-L248


This comment was generated by todo based on a TODO comment in 54a1fd5 in #229. cc @roblabla.

@roblabla roblabla requested a review from marysaka March 17, 2019 15:46
@roblabla roblabla force-pushed the buffers branch 2 times, most recently from 25fa8d8 to f3f808f Compare March 17, 2019 15:54
Copy link
Member

@marysaka marysaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@todo
Copy link

todo bot commented Mar 19, 2019

Use plain to ensure T is a valid POD type

Plain would give us two benefits: it would do the alignment and size checks for us, and it would give a type-system guarantee that our T is valid for any arbitrary type.


https://github.com/roblabla42/KFS/blob/e114a2be62c4e5d476d20213c26d5342933a3d2d/libuser/src/ipc/buffer.rs#L27-L37


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

@roblabla roblabla requested a review from Orycterope March 19, 2019 14:10
@marysaka marysaka self-requested a review March 19, 2019 15:03
roblabla added 10 commits March 19, 2019 17:51
We add a new function, share_existing_mapping, which turns an existing
Normal mapping into a Shared mapping, without removing it from the page
table, avoiding potential race conditions.
Introduce the InPointer and InBuffer types, which are smart pointers
representing a specific kind of IPC Buffer for an IPC Server.

Introduce two functions to pop those buffers: Message::pop_in_pointer
and Message::pop_in_buffer.

Add functions to push in and out pointers (type-C and type-X).
We do lots of shifting operations on the address and sizes.
Unfortunately, it's come to my attention that x1 >> x2 has a completely
broken behavior if x2 is higher than the bitwidth of x1 (instead of
returning the obvious 0). This breaks the implementation on 32-bit.

To remediate this, we simply move all the addresses and sizes to u64.
Instead of panicking in find_ty_cmdid, we return None. The server will
then return the appropriate error.
For now, the C Buffer size is hardcoded to 0x300. In the future, that
should probably be a const generic.
Check that the address/size of the x and buffer fits an usize.
Buffer should check that the address has the correct alignment for the
type T, otherwise we might get some nasty UB.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants