If you have reason to upload a new diff (e.g. addressing some feedback) please upload with full context - e.g. git diff -U99999 or git show -U99999 <hash>. Full details at https://wiki.freebsd.org/Phabricator

I read the description, but I'm still not sure how a jail descriptor would be used. How about some pseudo-code, to illustrate the concept please?

In D43696#996669, @dvl wrote:

I read the description, but I'm still not sure how a jail descriptor would be used. How about some pseudo-code, to illustrate the concept please?

From just looking at the diff in it's current form I assume the following:

• jail_set() with the JAIL_CREATE flag can be used to create a jail like before, but you can pass the new "desc" argument with a negative number (e.g. -1) as argument. This would cause jail_set() to attempt to create a new jail descriptor bound to the the jail and put into the calling process. The file descriptor number is returned by overwriting the previously negative int argument.
• jail_get() can be used the same way to get a jail descriptor for an already existing jail.
• jail_set() can than be used with the "desc" argument and a non-negative file descriptor number instead of the jail id in which case the permission checks are performed against the credentials used to create the jail file descriptor instead of the calling processes current permissions.
• Or it could be required to use the new syscalls for this.
• It's planned, but not yet implemented, to register a jail descriptor with kqueue using a new filter to select which types of events to notify (a variable was modified, a process attached, the jail was destroyed, etc.).
• Not mentioned but also missing to allow safe delegation to unprivileged users is a way to restrict the capability represented by a jail descriptor e.g. attach only, destroy only, watch only.

The __specialfd() system call is good to know about, but on its own doesn't provide a way to atomically create a configured jail with its lifetime tied to the file descriptor unlike passing a negative integer as "desc" argument to jail_set. There would either be a window where the jail would exist without a file descriptor associated with it which would reintroduce races and resource leak or the file descriptor has to start out unattached to a jail and be passed to the jail_set(JAIL_CREATE) call. The later design choice would get rid of the awkwardness of (ab-)using negative file descriptor numbers to request a file descriptor via an in-out parameter.

In D43696#996617, @bz wrote:

Also, can we please have a man page?

Of course I'll need to do that sometime ;-). The parameters to get/set the descriptor will have their place in jail(8) where all the other parameters live, though they're kind of strange there because it's very much not part of the "(8)" side of things. Also jail_attach_jd and jail_remove_jd (or whatever we settle on) could be added to jail(2) which has the other jail-related calls.

For how the whole thing works, I suppose a jaildesc(4) works, again as a counterpart to procdesc(4).

The syscall bits look good (module feedback on a flag argument which you can take or leave). I've not reviewed the descriptor bits in great detail so another review by someone more familiar with that code would be good.

@jamie this not longer builds against CURRENT, if possible can you massage it a little, and then commit it so we don't get any more rot? I fixed symbol.map changes in https://git.sr.ht/~dch/src/commit/9051eedd52fc but that's not sufficient.

In D43696#1179550, @jamie wrote:

The big addition is kevent support. A jail descriptor can pass four notes:

The way descriptors were passed to/from jail_set and jail_get has been updated with new flag bits, instead of the old hacky method of checking of checking whether the passed number is -1. The descriptor number is still passed via the "desc" parameter, but now the flag bits control how it's used. I've updated the summary to describe these bits.

One new use of the descriptor is the flag JAIL_AT_DESC, which uses the described jail as the current/parent prison, in place of the caller's prison. This allowed child jails to be manipulated using a descriptor, in the spirit of the "fooat" family of file system calls.

I've added file access permissions, via the mode bits on the descriptor, settable via fchmod. By default the read bits are on (as anyone can call jail_get) and the write and execute bits are on for the user and only if it has the associated privileges (jail_set for write, jail_attach for execute). I also use the sticky bit the make the jail go away when the descriptor closes, similar to the way a process is killed when its process descriptor closes.

libjail has been updated to be aware of the desc parameter and flags, since it needs to know when to pass it to jail_get and retrieve it from jail_set.

This sounds deeply confused about what a file descriptor is (supposed to be). It is the capability to use the referenced resource. The permissions in the file description should govern what's allowed through the descriptor not the current process credentials. The check against the current process credential has to happen as the description the descriptor references is created.

"When the descriptor closes, do will the attached jail" should probably read "... so will the attached jail".

In D43696#996669, @dvl wrote:

I read the description, but I'm still not sure how a jail descriptor would be used. How about some pseudo-code, to illustrate the concept please?

Now that the kevent stuff is there, you have to tools to handle keeping track of a jail hierarchy. Something like...

		my_jid = jail_setv(JAIL_CREATE | JAIL_OWN_DESC, "name", "foo", "desc", desc, ..., NULL);
		kq = kqueue();
		EV_SET(&ev, desc, EVFILT_JAILDESC, EV_ADD, NOTE_JAIL_SET | NOTE_JAIL_ATTACH | NOTE_JAIL_REMOVE | NOTE_JAIL_CREATE, 0, NULL);
		kevent(kq, &ev, 1, NULL, 0, NULL);
		for (;;) {
			if (kevent(kq, NULL, 0, &ev, 1, NULL) > 0)
				switch (ev.fflags) {
				case NOTE_JAIL_SET:
					jail_getv(JAIL_USE_DESC, "desc", desc, ..., NULL);
					do_something_with_jail(...);
					break;
				case NOTE_JAIL_ATTACH:
					do_something_with_process(ev.data);
					break;
				case NOTE_JAIL_REMOVE:
					close(desc);
					goto done;
				case NOTE_JAIL_CREATE:
					jail_getv(JAIL_GET_DESC, "jid", ev.data, "desc", "child_desc", "parent", parent_jid, ..., NULL);
					if (parent_jid != my_jid) {
						// ignore or handle race where this isn't
						// the jail that I just got a note for.
					} else
						do_something_with_jail(child_desc);
					break;
				}
			}
		}

Other features let different tasks be delegated to a sub-process or whatever.

In D43696#1179565, @crest_freebsd_rlwinm.de wrote:

In D43696#1179550, @jamie wrote:

This sounds deeply confused about what a file descriptor is (supposed to be). It is the capability to use the referenced resource. The permissions in the file description should govern what's allowed through the descriptor not the current process credentials. The check against the current process credential has to happen as the description the descriptor references is created.

Um, yeah. I'm just saying that's how it works now. In the previous revision, those checks weren't there, and anyone who had the descriptor could do anything with it. Whether it's "new feature" or "finally works correctly" is just different ways of describing the new code.

Or am I deeply confused enough that I don't know which part of my description you're referring to?

In D43696#1179566, @crest_freebsd_rlwinm.de wrote:

"When the descriptor closes, do will the attached jail" should probably read "... so will the attached jail".

Hey, I'm not the one who chose to put those keys right next to each other.

As I learn more about kqueue, I see I am trying to make it do things it's not suited for. In particular, the data field can't contain the kind of information I'm putting it in (process ID, jail ID). The non-queuing nature of kqueue (!) means that one event can be obliterated by the next event, leading to unreliable notification. There's not a lot of advantage of being notified that at least one child jail has been created, and here's the JID of the most recent one.

Notably all the existing uses of kqueue have data that tells you many times something happened, of how much of something exists - the kind of data where the latest value is the only useful one. An exception is EVFILT_PROC, where an exiting process gives you its status, and NOTE_TRACK gives you a NOTE_CHILD with a parent PID. The former works because NOTE_EXIT will always be the last thing a process does, and the latter because the kernel creates a new kevent behind the scenes (which may fail, leading to NOTE_TRACKERR).

So perhaps I could go the NOTE_TRACK route, especially considering the creating a child jail corresponds somewhat to a fork. That would include the caller having to handle a failure to be told.

I've added D51940, which adds kqueue support to jails by JID, so it's separate from the descriptor work, except it contains fixes for the problems I've identified with the current kqueue code in this revision. In particular, it uses the NOTE_TRACK convention that processes use, and adds a flag that notes if attached processes were missed.