[backport v2.2] Restore socket descriptor permission management #27175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change extends the parse_syscalls.py script to scan for a __subsystem sentinal added to driver api declarations. It thens generates a list that is passed into gen_kobject_list.py to extend the subsystems list. This allows subsystems to be declared in the code instead of a separate python list and provides a mechanism for defining out-of-tree subsystems. Signed-off-by: Corey Wharton <[email protected]>
Actual files make terrible dependency targets in CMake. Wrap the generation of subsystems.json into a custom target to get around this. Fixes a problem where parse_syscalls.py was being called multiple times. Fixes: zephyrproject-rtos#23504 Signed-off-by: Andrew Boie <[email protected]>
Rather than stuffing various values in a uintptr_t based on type using casts, use a union for this instead. No functional difference, but the semantics of the data member are now much clearer to the casual observer since it is now formally defined by this union. Signed-off-by: Andrew Boie <[email protected]>
Private type, internal to the kernel, not directly associated with any k_object_* APIs. Is the return value of z_object_find(). Rename to struct z_object. Signed-off-by: Andrew Boie <[email protected]>
This was supposed to match definitions if dynamic objects are turned on. Signed-off-by: Andrew Boie <[email protected]>
This was passing along _current->ssf, but these types of bad syscalls do not go through the z_mrsh mechanism and was passing stale data. We have the syscall stack frame already as an argument, propagate that so it works properly. Signed-off-by: Andrew Boie <[email protected]>
Certain types of system call validation may need to be pushed deeper in the implementation and not performed in the verification function. If such checks are only pertinent when the caller was from user mode, we need an API to detect this situation. This is implemented by having thread->syscall_frame be non-NULL only while a user system call is in progress. The template for the system call marshalling functions is changed to clear this value on exit. A test is added to prove that this works. Signed-off-by: Andrew Boie <[email protected]>
Instead of handling this ad hoc, generalize which kobject types can be allocated. Signed-off-by: Andrew Boie <[email protected]>
Now we can build up lists of data structures matching a list of particular tags, with __subsystem being just one case. Relax searches to also look inside C files, since struct prototypes may be declared there as well. Signed-off-by: Andrew Boie <[email protected]>
Any data structure declaration tagged with __net_socket will end up in the kernel object table with type K_OBJ_NET_SOCKET. These all correspond to objects which are associated with socket file descriptors and can handle the socket vtable API. Signed-off-by: Andrew Boie <[email protected]>
Used for permission validation when accessing the associated file descriptors from user mode. There often get defined in implementation code, expand the search to look in drivers/ and subsys/net/. Signed-off-by: Andrew Boie <[email protected]>
Zephyr running on MPU devices have a different memory model than process-oriented OSes like Linux and require a method to set kernel object permissions on a file descriptor's underlying context object. Add this, and a test to show that it is working. Signed-off-by: Andrew Boie <[email protected]>
This socket is shared by all the test cases which run in different threads. Just make it a global object here. Signed-off-by: Andrew Boie <[email protected]>
Add test case to prove that this new API works. Signed-off-by: Andrew Boie <[email protected]>
Anytime a file descriptor context object is updated, we need to reset its access permissions and initialization state. This is the most centralized place to do it. Signed-off-by: Andrew Boie <[email protected]>
This is just done in common code now. Signed-off-by: Andrew Boie <[email protected]>
We now have a low-level function z_dynamic_object_create() which is not a system call and is used for installing kernel objects that are not supported by k_object_alloc(). Checking for valid object type enumeration values moved completely to the implementation function. A few debug messages and comments were improved. Futexes and sys_mutexes are now properly excluded from dynamic generation. Signed-off-by: Andrew Boie <[email protected]>
The original sockets system calls used file descriptors which were actually net_context pointers. For all socket system calls, any calls from user mode would check if the caller had permission to use the net context. This was later changed to not stuff net_context pointers into file descriptors, but all the permission checking was unintentionally lost, allowing all threads on the system to read/write all socket file descriptors in the system at will, with no way to isolate applications running on the same microcontroller from each other's network activity. This patch restores the permission checks on network context objects for socket system calls that originated from user mode. The call to z_object_recycle() was never removed from zsock_socket_internal(); this is again leveraged to grant the caller who opened the socket permission on the net_context associated with the returned file descriptor. To ensure that all socket calls do this checking, all uses of z_get_fd_obj_and_vtable() have been routed through get_sock_vtable(). Objects have initialization state set and thread permissions reset to just the caller in common zsock_socket() code. Signed-off-by: Andrew Boie <[email protected]>
The kobject dict has three parameters now after the dynamic object patches. Signed-off-by: Jukka Rissanen <[email protected]>
jukkar
requested review from
carlescufi,
galak,
MaureenHelm,
mike-scott,
nashif,
pfalcon and
tbursztyka
as code owners
tbursztyka
approved these changes
Jul 27, 2020
github-actions
bot
added
area: API
|
@jukkar the milestone appears incorrect since 2.2.1 has already been released. Furthermore, to my understanding the general 2.2.x point releases stopped the moment that 2.3 was released. The only exception might be if there's a super critical vulnerability with an assigned CVE, but that doesn't appear to be the case here. |
andrewboie
approved these changes
Aug 10, 2020
|
This is assigned CVE-2020-10072, which left embargo 2020-08-28. |
|
v2.2 is out of scope for backports right now, closing. Use a more recent version. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #25804 to 2.2 branch.
Requires these commits from 2.3 not found in the original PR #25804
ccd15df
fd34aadb0800deb725818ca346d9d6917b1e1a3e
f2734ab
2dc2ecf
I also needed to create a small fix for
scripts/elf_helper.pyas that file is not found in 2.3 but it is in 2.2.I had to resolve several merge conflicts, and might have made mistakes during resolve. So this needs extra careful review.
Fixes #27176