Why does it matter that putenv(3) doesn't create a copy?

The original author should have done this in the first place ;-)

While I prefer the version I mentioned in the inline notes (it's a little less branchy), I'm also fine with the patch as originally given.

It only touches peripherally on jails, but sure, looks good. One could ask why we decided to have a separate and subtlely different "dist" and "distname" but I suppose that's water long since under the bridge.

Commit 66d8ffe30 has simpler kevent handling for jaildesc, without any recursion. Jail kevents have also have recussion removed. Its lack of guarantees and incomplete problem-solving meant that applications would need a way to handle when notifications weren't 100% collected. As long as that's the case, might as well get rid of the complexity.

Commit 66d8ffe30 has simpler kevent handling for jaildesc, without any recursion. Jail kevents have also have recussion removed. Its lack of guarantees and incomplete problem-solving meant that applications would need a way to handle when notifications weren't 100% collected. As long as that's the case, might as well get rid of the complexity.

Very well. I suppose it doesn't hurt.

Plan B is in D52462.

Plan A is in D52461.

Yes, EPERM on open directories may be unexpected, but I don't see that is enough cause to change the long-standing tradition of referring to other manual pages when a function may return the set of errors produced by another function.

I've added a "Jail Descriptors" section to jail(2), and added the jail_attach_fd and jail_remove_jd system calls, and the new jail_get/jail_set flags.

The unrelated prison_add_allow talk deserves its own revision, so I made D52105 for that.

In D52039#1189578, @markj wrote:

I wonder why asprintf() uses M_NOWAIT when strdup() uses M_WAITOK. Looking at the relatively few callers, I suspect we can perhaps just switch to M_WAITOK...

prison_add_vfs then goes on to call prison_add_allow, which has its own asprintf call. So unless you fix that as well, you've bought very little. And prison_add_allow really ought to be fixed. It's just a matter of not relying on asprintf with its M_NOWAIT allocation. The context it's called in (and presumably would ever be called in) can handle waiting for allocation.

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.

Yep, tried it out and it looks good now.

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.

True, there's no real need for the PR_ALLOW_PRISON0 change, but since you went to the effort to make that macro anyway, it's a good place to showcase it.

It looks fine from the jail side of things.

In D51645#1179838, @jamie wrote:

In the meantime (and long-term), a knob makes sense.

In D51645#1179733, @kib wrote:

Would privileges actually work, I have no objections. But right now this change makes the very useful feature (at least for me), only available to root. I do often use 'jail -u <me> / something 127.0.0.1 /bin/sh', and have the jailed processes only bound to localhost, otherwise they are normal (can be debugged etc).

It would be pity to loose the ability. Can we have at least a knob to re-enable the current behavior?

I like this. Given that a non-root user isn't allowed to mess with a jail, it makes that it wouldn't be allowed to mess with processes in that jail, even if they happen to have the same uid. That sounds like preferable default behavior, even if it's a switch from current practice. The closer we get to namespaces like uid being conceptually a (jail, id) tuple, the better off we are.

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.

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?

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

In D51524#1176869, @markj wrote:

I could instead add an alternative function or even just an inline "is PR_VNET set" check.

There's a general trend in the "prison can do this thing" functions that they all take a ucred. One the one hand it seems reasonable that they would all take a struct prison instead, but is there a particular reason to break the trend for this case?

I had considered that the -l (exec clean) flag should be considered, but decided it really only makes sense for keeping the jail environment clean.