Skip to content

Commit

Permalink
kern: reply buffer capacity checking
Browse files Browse the repository at this point in the history
The syscall docs say that attempting to reply with a message that's too
big for your client to accept is a programming error. Programming errors
are expected to produce faults in the program with the error. This fault
is documented as being of type `ReplyTooLarge`.

However, there is no such fault defined in the ABI, and the actual
behavior of the kernel is to deliver the prefix of the reply that fits,
and hand the client the length of that prefix. In our current world,
this means the client will generally die shortly thereafter with a
deserialization error.

In a potential future world where messages can contain variable length
portions, the client might not even hit an error when the reply is
truncated.

The code in the kernel has been marked with a TODO since forever. This
commit resolves the TODO by adding a check and fault reason.
  • Loading branch information
cbiffle committed Dec 14, 2024
1 parent 2c8ac9b commit 4200e2f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
8 changes: 5 additions & 3 deletions doc/syscalls.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ There is only one way to break `REPLY`, and that's with a bogus slice.
| `MemoryAccess`

| Reply message is longer than recipient requested.
| `ReplyTooLarge`
| `ReplyTooBig`

|===

Expand All @@ -346,8 +346,10 @@ Reply messages can be zero-length, in which case the base address of the slice
is ignored. Often, the response code is enough.

`RECV` delivers the size of the caller's response buffer, so your task has
sufficient information to not overflow it. This is why doing so is a fault: it's
a programming error.
sufficient information to not overflow it. If the caller's response buffer is
too small, you are expected to instead use `REPLY_FAULT` with the
`ReplyBufferTooSmall` code. If you instead send a reply that won't fit, that's
treated as a programming error in your task, and you take a fault.

[#sys_set_timer]
=== `SET_TIMER` (3)
Expand Down
4 changes: 4 additions & 0 deletions sys/abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,10 @@ pub enum UsageError {
BadKernelMessage,
BadReplyFaultReason,
NotSupervisor,

// A server is attempting to reply with a message that is too large for the
// client to handle.
ReplyTooBig,
}

/// Origin of a fault.
Expand Down
13 changes: 9 additions & 4 deletions sys/kern/src/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,16 @@ fn reply(tasks: &mut [Task], caller: usize) -> Result<NextTask, FaultInfo> {
}
};

// Validate that the reply fits in the recipient's buffer. Servers are
// expected to get this right, because the reply size is delivered along
// with the message: if the client didn't provide enough space, the server
// should reply-fault instead of replying. So, we assume any reply that
// would be truncated is a server bug.
if src_slice.len() > dest_slice.len() {
return Err(FaultInfo::SyscallUsage(UsageError::ReplyTooBig));
}

// Okay, ready to attempt the copy.
// TODO: we want to treat any attempt to copy more than will fit as a fault
// in the task that is replying, because it knows how big the target buffer
// is and is expected to respect that. This is not currently implemented --
// currently you'll get the prefix.
let amount_copied = safe_copy(tasks, caller, src_slice, callee, dest_slice);
let amount_copied = match amount_copied {
Ok(n) => n,
Expand Down

0 comments on commit 4200e2f

Please sign in to comment.