Only skimmed over it, but seems good except for one typo.

Don't have time to review this in detail, but overall you have my blessing as well. :-)

I'd put all new functions of sys/security/mac/mac_syscalls.c into sys/security/mac/mac_prison.c instead, as these are not really system calls, and export mac_label_copyin_string() from the former.

There is an alloc/free mismatch, see inline comment. Else seems good.

In D53954#1239740, @kevans wrote:

Ok, thanks- to be clear, I'm still not against the idea of providing an abstraction layer to avoid some future headache for MAC modules when the core implementation changes. Did you have some sketch of what you're thinking in the area, by chance?

There's a compile error (missing parenthesis, see inline comment).

Feels much better like that. I have one more inline comment for a change in sysctl_epp_select_per_core(). With that, you can consider the changes reviewed!

In D53954#1239729, @kevans wrote:

I'm not sure I understand this last bit

In D53954#1239668, @kevans wrote:

I'm not too worried about that, though it does occur to me that it probably makes sense to stop setting an error in the opts for any of those checks. The MAC policy should probably have free reign over both errno and error probably in case it's some kind of custom vendor integration.

Compile errors because of leftovers of some prison_copy_label() operation.

While I'm generally supportive of adding a more discoverable alternative to the standard tail -r (as expressed elsewhere), and applaud your constant efforts on test cases, I find the proposed implementation problematic:

• It's an entirely separate program whereas it's basically a combination of functionality that exists in tail and in cat (more details below). It's only ~100 C lines, but it may be 90 lines too many (see again below).
• It sucks a file entirely in memory before outputting anything, so is not friendly to big files, and even less to pipes.
• It uses fread() where it could use mmap() (doesn't matter for regular files now, but will once the previous point is fixed).
• It's not CAPSICUMized.

Coming back to highlights, there's indeed still the problem that enumeration returns an error if the last jail is denied via mac_prison_check_get(), so doing what you suggest is probably mandatory: Put back the MAC check in the loop, and once a jail is returned, jump to a prison_found_nomac label, even a _nomac_noalive variant as to eliminate also the (harmless) redundant test.

In D53953#1235126, @kevans wrote:

Highlights:
(snip)

Modulo code duplication, that's fine. And thanks for adding a test.

Code looks correct (more precisely, not worse than the existing), but some effort to factor out at least part of the preambles of both the new unionfs_copylink() and the existing unionfs_copyfile(), and both of the new unionfs_vn_symlink_on_upper() and the existing unionfs_vn_create_on_upper(), respectively, would be appreciated.

In D49587#1238151, @aokblast wrote:

Thanks for the comments! Hope it looks better now.

In D53954#1238908, @kevans wrote:

Ping

A small clarification: Above, I've been talking both to possibly remove struct hwpstate_cppc_setting (=> struct hwpstate_cppc_state) and to move the req field to it, which is not apparently consistent. I was considering both separately. The consistent view is that req should be moved to struct hwpstate_cppc_state and keep the latter unconditionally but possible removing from it the high, guaranteed, efficient and low fields (depending on some choices described above). req has to be kept in all cases as it is needed in sysctl_epp_select() to return the current value (which here is probably the proper thing to do; in the debug knob it's debatable whether we actually want to re-read from the MSR).

In D53988#1237096, @jah wrote:

I made exactly this change in my original heavyweight attempt at unionfs_getlowvnode (which did do the copy-up and thus required the vnode lock). But as a practical concern, I think the nullfs issue you mentioned above could be just as easily dealt with by changing null_getlowvnode() to take the interlock instead of the full vnode lock. We might still want to change this contract anyway, but I think that should be done as a separate change, no?

I don't think just having unionfs_getlowvnode() return EACCESS alone is enough, it has problems, see inline comment.

So, the main question now is whether we keep struct hwpstate_cppc_setting (=> struct hwpstate_cppc_state) at all, see inline comments.

In D49587#1235859, @aokblast wrote:

Is it what you mean? Use the provided sysctl to control the package level or logical cpu level control for cppc but Implement package level control in software? If that is the case, it is just like the intel one now.

In D49587#1235740, @aokblast wrote:

@olce I think exposing the epp setting interface only in only cpu0 makes sence and therefore I make some changes.

Haven't reviewed the constraints code yet, and some of the device plumbing.

Thanks.

There is an occurrence of "swapper" in sys/vm/vnode_pager.c that seems to also point to the swapper process.

(Unfinished.)

For the missing unlocks.

If the MAC policies must be able to hide jails, then this case should be completely indistinguishable from the "the jail doesn't exist" one, which means returning EINVAL and the exact same error message, which varies depending on the code point.

I guess that what protects the prison's label is the jail's mutex. That explains why you use the MAC_POLICY_CHECK_NOSLEEP() variant almost exclusively. This needs at least some documentation, perhaps just in the form of assertions that the prison is locked in the different entry points.

As said in the previous comment, sorting commits by chronological order could be better.

In D54013#1234041, @ziaee wrote:

+ use commas to separate gitrefs. I can do the rest of page if good?