Skip to content

Conversation

@Ticonderoga2017
Copy link
Contributor

#39

Description

  • Remove UserPtr/UserConstPtr and their supporting memory validation functions to implement a unified user-space memory access interface.
  • Introduce primitives like VmPtr/VmMutPtr, vm_read_slice, vm_write_slice, and vm_load_until_nul from starry_vm, covering scenarios such as read/write operations and string loading.
  • Uniformly adapt system call paths and file I/O interfaces to the axio framework's Read/Write and IoBuf/IoBufMut models, avoiding inconsistencies caused by custom pointer types.

@AsakuraMizu
Copy link
Contributor

Good job 👍

Copy link
Contributor

@AsakuraMizu AsakuraMizu left a comment

Choose a reason for hiding this comment

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

Sorry, there were too many files, and I haven't had time to look at a few of them myself.

Comment on lines +129 to +131
let mut len = addrlen.vm_read()?;
remote_addr.write_to_user(addr, &mut len)?;
addrlen.vm_write(len)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't do this. I already suggested you to change the signature of SocketAddrExt::write_to_user.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the codebase to remove custom UserPtr and UserConstPtr types in favor of a unified memory access interface from starry_vm. The changes introduce VmPtr/VmMutPtr primitives and helper functions like vm_read_slice, vm_write_slice, and vm_load_until_nul for safer user-space memory operations.

Key Changes:

  • Replaced all UserPtr/UserConstPtr usages with raw pointers and starry_vm primitives
  • Updated system call signatures to accept raw pointers instead of custom wrapper types
  • Refactored CMsgBuilder to work with raw pointers and return Result from constructor
  • Simplified syscall dispatcher by replacing .into() calls with as _ casts

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
api/src/mm.rs Removed UserPtr/UserConstPtr implementations; kept VmBytes/VmBytesMut wrappers and vm_load_string helper
api/src/vfs/dev/event.rs Converted event device ioctls to use vm_write_slice and allocate temporary buffers
api/src/syscall/net/socket.rs Updated socket syscalls to use raw pointers with VmPtr/VmMutPtr traits
api/src/syscall/net/opt.rs Refactored getsockopt/setsockopt to use vm_read/vm_write with temporary variables
api/src/syscall/net/name.rs Simplified getsockname/getpeername to use raw pointers
api/src/syscall/net/io.rs Updated sendmsg/recvmsg to use vm_read_uninit and addr_of_mut for field access
api/src/syscall/net/cmsg.rs Refactored CMsgBuilder to use raw pointers and made constructor fallible
api/src/syscall/mod.rs Replaced .into() conversions with as _ casts throughout syscall dispatcher
api/src/syscall/ipc/shm.rs Updated shmctl to use vm_read/vm_write with nullable pointer handling
api/src/syscall/io_mpx/select.rs Converted select/pselect6 to read/write local copies and use nullable pattern
api/src/syscall/io_mpx/poll.rs Updated poll/ppoll to use local Vec buffers with vm_read/vm_write
api/src/syscall/io_mpx/epoll.rs Converted epoll functions to use temporary buffers and vm_write
api/src/syscall/fs/memfd.rs Changed memfd_create signature to use raw pointer
api/src/syscall/fs/io.rs Updated truncate to use vm_load_string
api/src/syscall/fs/fd_ops.rs Modified fcntl to use vm_read_uninit/vm_write for flock64
api/src/socket.rs Updated SocketAddrExt trait to use raw pointers; refactored read/write methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

addr.cast::<u8>()
.get_as_mut_slice(len)?
.copy_from_slice(&data[..len]);
let _ = vm_write_slice(addr.cast::<u8>(), &data[..len]);
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The function vm_write_slice can fail and return an error, but the result is being ignored here with let _. This could silently fail to write the socket address data to user space, which would be a serious bug. The error should be propagated or handled appropriately.

Suggested change
let _ = vm_write_slice(addr.cast::<u8>(), &data[..len]);
vm_write_slice(addr.cast::<u8>(), &data[..len])?;

Copilot uses AI. Check for mistakes.
for (i, it) in local.iter().enumerate() {
unsafe { fds.add(i).vm_write(*it)? };
}
// do_poll(fds, timeout, nullable!(sigmask.get_as_ref())?.copied())
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The commented-out line at the end suggests there may be incomplete refactoring. Either this comment should be removed if the refactoring is complete, or the old code should be restored if the new implementation is not yet validated.

Suggested change
// do_poll(fds, timeout, nullable!(sigmask.get_as_ref())?.copied())

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants