restore socket descriptor permission management

[andrewboie]

A change made to the socket system calls to use actual file descriptors (instead of struct net_context stuffed into an integer) unintentionally lost the ability to manage per-thread permissions of file descriptors, which is needed in MPU-based user mode memory models (currently all arches that implement user mode in Zephyr).

This PR:

• Adds an API z_is_in_user_syscall() so that system call verification checks may be done deep within implementation functions if those checks are only relevant to syscalls originating from user mode.
• All network context object types which have __net_socket in their declaration will have their instances tracked as kernel objects with type K_OBJ_NET_SOCKET
• Socket APIs once again check for object permissions after the object is retrieved from file descriptor tables. This check only applies if the system call originated from user mode.
• Add a custom API that, given a file descriptor, returns a pointer to the associated struct net_context so that APIs like k_object_access_grant() may be used.
• Add tests for new APIs, and show that permission management is working as expected across user threads.
• Add some infrastructure for registering K_OBJ_NET_SOCKET objects dynamically, needed by the socketpair implementation (which pulls data from a heap instead of a pool like other socket types).

Please review individual commit messages for more information.

Process-based virtual memory is coming to Zephyr for systems with an MMU later this year. In the fullness of time, we can adopt file descriptors that have process-level scope (instead of global scope), removing the need for this management on those types of systems. We will likely continue to need this infrastructure indefinitely for MPU-based systems, but it is the same permission model we already use for all kernel objects and device driver instances.

[andrewboie]

@jukkar I have this almost working.

It works as expected for regular native sockets, as they are all backed by a struct net_context which I've restored as a kernel object to track permissions.

It doesn't work for registered socket families with NET_SOCKET_REGISTER(), as these are not backed by a struct net_context that I can see, instead custom data structures which are dependent on the implementation. I didn't see this until I started trying to run tests that use them like tests/net/socket/net_mgmt.

I've just now started looking at this part and will hopefully come up with a generic solution.

[andrewboie]

@jukkar think I found something workable.

There's no common object type I can use for underlying socket descriptor objects, so it's not as simple as the last time I looked in this code, where you could make net_context a kernel object and call it a day.

Any given socket registered with NET_SOCKET_REGISTER() will have their own object type that is returned when you call z_get_fd_obj_and_vtable(). It's an arbitrary void * type.

However, I can use a technique that was contributed to make driver disambiguation possible, because in order to distinguish drivers, we needed to be able to identify what subsystem it belongs to. If you look through subsystem headers you'll see that the API struct definition for all subsystems is tagged with __subsystem in order to identify it. The identity of the API struct is how we distinguish between driver types and forbid the user from making, for example, UART API calls on a I2C driver object.

I want to try to do something similar to tag socket descriptor object type struct definitions with __net_socket_object or something similar. Things like struct modem_socket, struct net_mgmt_socket, etc, basically any struct which represents a socket fd object would be tagged with this.

Then the kernel object tables will have entries for any struct instance it finds that was tagged with __net_socket_object, and they will all have the same kernel object type that we can check. The fact that they all have the same object type signifies that they are compatible with methods in struct socket_op_vtable and are (from the core socket code's perspective) interchangeable.

We'll be able to handle any kind of socket object then.

Later on, could probably extend this technique to other fd types. It would need some notion of object type inheritance, but very doable.

[jukkar]

The tests look good. I wish we had these originally so would not have missed the issue.
We probably also need to add tests to other socket types like net_mgmt, websocket etc.

[andrewboie]

Agreed, I think the end goal we should strive for is to exercise the cross product of all socket APIs against all socket types.

[andrewboie]

Instead, there should be thread-specific execution (and thus, permission) context associated with a thread even after entering kernel, and all relevant native API functions in kernel should perform permission checks as they get called. And the idea to "snapshot the world" and perform all possible checks on entering the kernel seems rather cumbersome and rather flaky in the first place.

It's really quite breathtaking how forcefully you weigh your opinions on topics that you are so obviously uninformed on. Go read about security features like SMEP/SMAP, or contemplate the fundamental differences and constraints on handling between user vs kernel pointers if you don't understand why we are "snapshotting the world" upon entry/exit to system calls.

While you're doing that, given that you're the one who broke this to begin with, maybe you could lay off the trolling at least in this PR?

[pfalcon]

@andrewboie

handling between user vs kernel pointers

My questions in #25443 (comment) were exactly about details of validating user vs kernel pointers. And you refused (or were unable) to answer them.

While you're doing that, given that you're the one who broke this to begin with, maybe you could lay off the trolling at least in this PR?

I'm trying to share my load of maintenance of this stuff, and learn as I go. If you're not interested, or unable, to cooperate, and discuss various aspects of this matter, then feel to maintain and update it yourself as various subsystems evolve (and apparently accuse people of breaking it).

[carlescufi]

Merging this now so it makes 2.3.0-rc2

[pfalcon]

@carlescufi, @galak: FYI, this failed build in our CI for gnuarmemb toolchain (but not Zephyr SDK). The overall picture: https://ci.linaro.org/job/zephyr-upstream/5245/ . Example of failed build: https://ci.linaro.org/job/zephyr-upstream/5245/PLATFORM=frdm_k64f,ZEPHYR_TOOLCHAIN_VARIANT=gnuarmemb,label=docker-xenial-amd64-13/console . The error:

/home/buildslave/workspace/zephyr-upstream/zephyr/kernel/userspace.c: In function 'z_impl_k_object_alloc':
/home/buildslave/workspace/zephyr-upstream/zephyr/kernel/userspace.c:319:22: error: 'tidx' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   zo->data.thread_id = tidx;
   ~~~~~~~~~~~~~~~~~~~^~~~~~
/home/buildslave/workspace/zephyr-upstream/zephyr/kernel/userspace.c:285:12: note: 'tidx' was declared here
  uintptr_t tidx;
            ^~~~
cc1: all warnings being treated as errors

[zephyrbot]

The backport to v1.14-branch failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-v1.14-branch v1.14-branch
# Navigate to the new working tree
cd .worktrees/backport-v1.14-branch
# Create a new branch
git switch --create backport-25804-to-v1.14-branch
# Cherry-pick the merged commits of this pull request and resolve the conflicts
git cherry-pick c951d71ebaff67b6d33c57b6fa7e5d47bcffa4b8~15..c951d71ebaff67b6d33c57b6fa7e5d47bcffa4b8
# Push it to GitHub
git push --set-upstream origin backport-25804-to-v1.14-branch
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-v1.14-branch

Then, create a pull request where the base branch is v1.14-branch and the compare/head branch is backport-25804-to-v1.14-branch.