clarify system call verification data copying policies #23974
Conversation
Handler functions are now referred to as verification functions, update documentation to reflect this. Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Show best practices when handling parameters passed to system calls by reference. Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
andrewboie
requested review from
andyross,
ceolin,
d3zd3z,
dcpleung,
dleach02 and
ioannisg
|
One detail I'm not sure of: what is Zephyr's polices on VLAs? I know we have a couple verifiers that use them. |
andrewboie
changed the title
|
I'd say we should not have VLAs, there are unspecified and undefined behaviours associated with VLAs. e.g: when the size is 0 or negative Also, it is undefined what happens when the allocation fails and there is not way to figure it out. MISRA-C 18.8 requires to no use VLA as well |
| as a block of memory to write to some hardware destination, this data | ||
| must be read without any processing. No conditional logic can be implemented | ||
| due to the data buffer's contents. If such logic is required a copy must be | ||
| made. |
| verification function, and only perform parameter checks on the copies, which | ||
| user threads will never have access to. The implementation functions get passed | ||
| the copy and not the original data sent by the user. The | ||
| :c:func:`z_user_to_copy()` and :c:func:`z_user_from_copy()` APIs exist for |
There was a problem hiding this comment.
These names are kind of confusing. Any reason to not use z_copy_to_user and z_copy_from_user?
There was a problem hiding this comment.
these APIs already exist, and Zephyr naming convention (not followed 100%) is to have the verb last.
@d3zd3z this is a serious assertion. We have a threat model posted here: https://docs.zephyrproject.org/latest/reference/usermode/index.html. This is the first time anyone has communicated that this is insufficient. Please provide specific, actionable feedback on what is ambiguous or missing as soon as possible. Why have you not spoken about this before?? |
My sincerest apologies for the unbased claim. I was not aware of the threat model. |
Expand on policies on what to do with parameters passed to system calls by reference.
Security people, please feel free to throw spitballs at this.
The special case for large data buffers I think is needed, although it will require some care in writing the implementation functions. Otherwise, there will be the overhead of both a heap allocation to duplicate the buffer and an extra copy operation. I am particularly averse to requiring a heap for what is a very common operation in syscalls.
Also, what used to be called "handler functions" are now called "verification functions", update docs for this.