Page MenuHomeFreeBSD

Implement CloudABI's exec() call.
ClosedPublic

Authored by ed on Jul 14 2015, 1:32 PM.

Details

Summary

In a runtime that is purely based on capability-based security, there is
a strong emphasis on how programs start their execution. We need to make
sure that we execute an new program with an exact set of file
descriptors, ensuring that credentials are not leaked into the process
accidentally.

Providing the right file descriptors is just half the problem. There
also needs to be a framework in place that gives meaning to these file
descriptors. How does a CloudABI mail server know which of the file
descriptors corresponds to the socket that receives incoming emails?
Furthermore, how will this mail server acquire its configuration
parameters, as it cannot open a configuration file from a global path on
disk?

CloudABI solves this problem by replacing traditional string command
line arguments by tree-like data structure consisting of scalars,
sequences and mappings (similar to YAML/JSON). In this structure, file
descriptors are treated as a first-class citizen. When calling exec(),
file descriptors are passed on to the new executable if and only if they
are referenced from this tree structure. See the cloudabi-run(1) man
page for more details and examples (sysutils/cloudabi-utils).

Fortunately, the kernel does not need to care about this tree structure
at all. The C library is responsible for serializing and deserializing,
but also for extracting the list of referenced file descriptors. The
system call only receives a copy of the serialized data and a layout of
what the new file descriptor table should look like:

int proc_exec(int execfd, const void *data, size_t datalen, const int *fds,
          size_t fdslen);

This change introduces a set of fd*_remapped() functions:

  • fdcopy_remapped() pulls a copy of a file descriptor table, remapping all of the file descriptors according to the provided mapping table.
  • fdinstall_remapped() replaces the file descriptor table of the process by the copy created by fdcopy_remapped().
  • fdescfree_remapped() frees the table in case we aborted before fdinstall_remapped().

We then add a function exec_copyin_data_fds() that builds on top these
functions. It copies in the data and constructs a new remapped file
descriptor. This is used by cloudabi_sys_proc_exec().

Test Plan

cloudabi-run(1) is capable of spawning processes successfully, providing
it data and file descriptors. procstat -f seems to confirm all is good.
Regular FreeBSD processes also work properly.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ed retitled this revision from to Implement CloudABI's exec() call..Jul 14 2015, 1:32 PM
ed updated this object.
ed edited the test plan for this revision. (Show Details)
ed added reviewers: kib, mjg.
ed updated this revision to Diff 6923.
ed updated this object.Jul 14 2015, 1:46 PM
mjg edited edge metadata.Jul 15 2015, 6:31 PM
mjg accepted this revision.
fdescfree_remapped() shares some code with fdescfree(), but doesn't change reference counts, acquires no locks, etc. I could add something like do_fdescfree() that contains all the shared code.

This code is awaiting a surgery splitting struct filedesc and some other stuff, so it is fine enough to commit it in the current state as the code is going to be reworked soon.

In exec_copyin_data_fds() I look at fd_lastfile to ensure that there is a limit to the amount of memory that gets allocated. My guess that this field is supposed to be private. What would be the cleanest way to do this?

fd_lastfile is already used outside of kern_descrip, e.g. in select(). In fact if it was not for select, i would likely get rid of it. I may still. For the time being, it is perfectly fine to use it.

Is the call to closef() in fdescfree_remapped() all right?

I presume you ask if you should skip unlocking advisory locks. It is fine.

That said, I think the code is fine enough to hit the tree, but note that a lot of kern_descrip.c will be reworked in the near future.

The following bugs me though: with fdp passed to execve, O_CLOEXEC is not being honored, which may or may not be fine. If this is a deliberate action, a comment explaining it is in order.

This revision is now accepted and ready to land.Jul 15 2015, 6:31 PM
ed added a comment.Jul 16 2015, 7:00 AM
In D3079#61304, @mjg wrote:

The following bugs me though: with fdp passed to execve, O_CLOEXEC is not being honored, which may or may not be fine. If this is a deliberate action, a comment explaining it is in order.

Good point. Due to the way CloudABI's exec() is designed, there is no way you can influence the close-on-exec function. This function simply ignores its value. I've extended the comment above fdcopy_remapped() to clarify this.

Thanks for reviewing this!

ed edited edge metadata.Jul 16 2015, 7:05 AM
ed updated this revision to Diff 6997.

Update comment to reflect that close-on-exec is ignored.

This revision now requires review to proceed.Jul 16 2015, 7:05 AM
ed updated this object.Jul 16 2015, 7:05 AM
ed edited edge metadata.
This revision was automatically updated to reflect the committed changes.