MAC/priority module for realtime privilege group

[dev_submerge.ch]

Summary

This is a MAC policy module that grants scheduling privileges based on group membership. Users or processes in the group realtime (gid 47) are allowed to run threads and processes with realtime scheduling priority. For timing-sensitive, low-latency software like audio/jack, running with realtime priority helps to avoid stutter and gaps. The idea came out of a discussion about a bug report.

One thing to keep in mind, is that currently both idle and realtime priority privileges are granted through the PRIV_SCHED_RTPRIO kernel privilege. There is no corresponding PRIV_SCHED_IDPRIO privilege, thus members of the realtime group are implicitly allowed to run processes at idle priority too. Opinions on that?

Test Plan

Tested on both 13.0-RELEASE and 14.0-CURRENT:

• mac_priority kernel module builds and loads
• Members of the realtime group can run processes at realtime priority
• Non-members of the realtime group can not do that
• Enable / disable functionality through sysctl security.mac.priority.realtime
• Change of group id through sysctl security.mac.priority.realtime_gid is effective

[kib]

Did you considered adding the same knob for the idle priority? security.bsd.unprivileged_idprio sysctl could be at least deprecated then, and even removed in HEAD.

NB. Please use 'diff -U 99999999' for patch generation, so that the context in review is useful.

share/man/man4/mac_sched.4

24 ↗

(On Diff #99239)

No point in adding svn tags.

sys/conf/files

5087

I suggest to call it more specific, e.g. mac_rt_sched. I suspect that somebody might want to use mac_sched name for something more generic, one day.

sys/security/mac_sched/mac_sched.c

47 ↗

(On Diff #99239)

The doc string should go into new line. It probably would be worth slightly expand on the description, like, 'enable realtime policy for group realtime_gid', or like that.

49 ↗

(On Diff #99239)

Please add GID_RT_PRIO to sys/conf.h (you would see the block) and use the symbolic constant there.

[dev_submerge.ch]

Changes:

• Extended diff context
• Subversion keywords removed from new files
• Group id defined in sys/conf.h

[dev_submerge.ch]

In D33191#750467, @kib wrote:

Did you considered adding the same knob for the idle priority? security.bsd.unprivileged_idprio sysctl could be at least deprecated then, and even removed in HEAD.

I can do that, but I think it only makes sense if we add a PRIV_SCHED_IDPRIO kernel privilege, see the second paragraph in the summary.
AFAICT that involves:

• Add PRIV_SCHED_IDPRIO to sys/priv.h (kernel module ABI change)
• Separate idle priority checks from realtime in kern/kern_resource.c
• Update rtprio man pages
• Deprecate security.bsd.unprivileged_idprio? Is there a mechanism for that?

Am I on the right track here?
Also I think that mac_sched as a module name is justified if we add the idle priority, it's easy to extend anyway, but I'm not gonna argue if you insist.

[kib]

In D33191#751252, @dev_submerge.ch wrote:

In D33191#750467, @kib wrote:

Did you considered adding the same knob for the idle priority? security.bsd.unprivileged_idprio sysctl could be at least deprecated then, and even removed in HEAD.

I can do that, but I think it only makes sense if we add a PRIV_SCHED_IDPRIO kernel privilege, see the second paragraph in the summary.
AFAICT that involves:

• Add PRIV_SCHED_IDPRIO to sys/priv.h (kernel module ABI change)
• Separate idle priority checks from realtime in kern/kern_resource.c
• Update rtprio man pages
• Deprecate security.bsd.unprivileged_idprio? Is there a mechanism for that?

Am I on the right track here?

Yes, the plan makes sense, but lets finish with the current change as is first.

Also I think that mac_sched as a module name is justified if we add the idle priority, it's easy to extend anyway, but I'm not gonna argue if you insist.

I still think that mac_prio or mac_sched_prio are better names, because they are more specific.

When finished with notes, please prepare the patch as git patch, most importantly, set the author metadata. Or specify which full name/email address should I use for Author.

sys/security/mac_sched/mac_sched.c

65 ↗

(On Diff #99372)

I think this could be written much more compact and easier to read as

if (priv == PRIV_SCHED_RTPRIO && realtime_enabled && groupmember(realtime_gid, cred))
        return (0);
return (EPERM);

69 ↗

(On Diff #99372)

Put '{' on the previous line.

[dev_submerge.ch]

Changes:

• Rename module to mac_priority
• Cosmetic changes as suggested

[dev_submerge.ch]

In D33191#751760, @kib wrote:

I still think that mac_prio or mac_sched_prio are better names, because they are more specific.

I settled on mac_priority, which is more in line with the other (unabbreviated) MAC module names. Hope this is ok with you?

When finished with notes, please prepare the patch as git patch, most importantly, set the author metadata. Or specify which full name/email address should I use for Author.

The patch is created with git format-patch --unified=99999999 now. That should include the metadata, but just in case:

Author: Florian Walpen <[e-mail]>
Date:   Tue Nov 23 13:14:24 2021 +0100

Thanks for your patience and guidance.

[kib]

There were some minor missed bits, like the MAC_PRIORITY option needs to be added to conf/options, and man pages date bump.

I am currently running tinderbox with the patch applied, will commit after the build finish.

Thank you for the submission. Consider doing a follow-up by also handling idle priority, in a similar manner.