I'm not a big fan of such a change.

Capsicum is about eliminating global state at the syscall level. This change is sort of compensating for it by adding more global state to the C library. In my opinion the correct change would be to finally add a thread-safe utmpx API where you can create handles to utmpx databases:

db = setutxent_r();
cap_enter();
while ((e = getutxent_r(db)) != NULL) {
    ...
}

In D7998#165666, @ed wrote:

I'm not a big fan of such a change.

Capsicum is about eliminating global state at the syscall level. This change is sort of compensating for it by adding more global state to the C library.

The trouble is we have an existing legacy library which already has a lot of global state. I don't think the additional global state in this change is any worse than the existing bunch.

In my opinion the correct change would be to finally add a thread-safe utmpx API where you can create handles to utmpx databases:

db = setutxent_r();
cap_enter();
while ((e = getutxent_r(db)) != NULL) {
    ...
}

Nothing about this change precludes doing what you propose later.

I don't have the time, experience with utmpx, or motivation to:

1. Add such a new API and negotiate it through inevitable bikeshedding of adding API surface to libc.
2. Convert every program that uses utmpx, many of which are ports, to the new FreeBSD-only API.

If you would like to do (1), I guess I would be more convinced that it is the right thing to do.

On the other hand — utmpx is hardly the only component of libc with a bunch of global state. Are you proposing to rewrite all of the nsswitch stuff, DNS resolvers, etc?

In D7998#165667, @cem wrote:

Are you proposing to rewrite all of the nsswitch stuff, DNS resolvers, etc?

Well, that's already what we're doing in some other cases, right? Casper already provides custom APIs for passwd, DNS, etc. that can be used while sandboxed.

Maybe we should just add utmpx support to Casper instead?

In D7998#165674, @ed wrote:

In D7998#165667, @cem wrote:

Are you proposing to rewrite all of the nsswitch stuff, DNS resolvers, etc?

Well, that's already what we're doing in some other cases, right? Casper already provides custom APIs for passwd, DNS, etc. that can be used while sandboxed.

Maybe we should just add utmpx support to Casper instead?

I think we should avoid the overhead of an extra daemon when we don't need it, such as for the utx stuff.

In D7998#165798, @cem wrote:

I think we should avoid the overhead of an extra daemon when we don't need it, such as for the utx stuff.

I don't agree to this reasoning. Why did we decide to put DNS, passwd and grp in Casper then?

In my opinion this makes things less consistent than before. There also doesn't seem to be any mechanism in place to force closing the varlog_fd and varrun_fd. For the tools that you are planning on sandboxing you also don't need this change. They only open a single database on startup, right?

In D7998#166376, @ed wrote:

In D7998#165798, @cem wrote:

I think we should avoid the overhead of an extra daemon when we don't need it, such as for the utx stuff.

I don't agree to this reasoning. Why did we decide to put DNS, passwd and grp in Casper then?

For DNS, we have no other choice.

Passwd and grp, I don't know the details. But if we can avoid the overhead of an extra daemon, I think we should reconsider Casper for these uses.

In my opinion this makes things less consistent than before.

Any change to behavior will do that :-). We could perhaps open a readonly root fd instead and use that consistently. But it seems like most accesses to utmp don't depend on arbitrary file access so it would be nice to restrict it to the common directories.

There also doesn't seem to be any mechanism in place to force closing the varlog_fd and varrun_fd.

Correct.

For the tools that you are planning on sandboxing you also don't need this change. They only open a single database on startup, right?

One tool reopens the same database partway through execution. In that case we would have to cap_enter much later without this change.

utmpx is kind of a crap API. It lacks a rewind functionality, it depends on a lot of global state, and lacks the ability to pass in open fds for arbitrary file handling. Maybe we should figure out the details of a nicer API we want, implement that, and adapt programs to it.

In D7998#166389, @cem wrote:

utmpx is kind of a crap API. It lacks a rewind functionality, it depends on a lot of global state, and lacks the ability to pass in open fds for arbitrary file handling. Maybe we should figure out the details of a nicer API we want, implement that, and adapt programs to it.

Exactly! :-)