sched_setscheduler(2): Fix realtime privilege check

[dev_submerge.ch]

Summary

Change the privilege checked from PRIV_SCHED_SET (set thread scheduler)
to PRIV_SCHED_SETPOLICY (set scheduling policy). This reflects what the
system call actually does, and allows users with realtime privilege (see
mac_priority(4)) to set scheduler policy through sched_setscheduler(2),
same as for other POSIX system calls (e.g. pthread_setschedparam(3)).

PR: 276962
Reported by: Jan Beich

[dev_submerge.ch]

Taken here from PR 276962.

sys/sys/priv.h defines:

#define  PRIV_SCHED_DIFFCRED     200     /* Exempt scheduling other users. */
#define  PRIV_SCHED_SETPRIORITY  201     /* Can set lower nice value for proc. */
#define  PRIV_SCHED_RTPRIO       202     /* Can set real time scheduling. */
#define  PRIV_SCHED_SETPOLICY    203     /* Can set scheduler policy. */
#define  PRIV_SCHED_SET          204     /* Can set thread scheduler. */
#define  PRIV_SCHED_SETPARAM     205     /* Can set thread scheduler params. */
#define  PRIV_SCHED_CPUSET       206     /* Can manipulate cpusets. */
#define  PRIV_SCHED_CPUSET_INTR  207     /* Can adjust IRQ to CPU binding. */
#define  PRIV_SCHED_IDPRIO       208     /* Can set idle time scheduling. */

mac_priority(4) grants PRIV_SCHED_RTPRIO and PRIV_SCHED_SETPOLICY to users in the realtime group. This led to a mismatch with kern_sched_setscheduler() checking for the PRIV_SCHED_SET privilege.
I think PRIV_SCHED_SETPOLICY fits better here because that's what the syscall actually does.

Tested with the test_sched.c example program from the PR. The privileges of root are unaffected.

Impact: kern_sched_setscheduler() is called from sched_setscheduler(2) through both FreeBSD native and Linux emulation syscalls. I assume privileges should apply the same for Linux emulation.

Disclaimer: I don't know where PRIV_SCHED_SET comes from, it is currently unused AFAICT. The overall approach is now to check for PRIV_SCHED_RTPRIO or PRIV_SCHED_IDPRIO where these cases are handled separately, and PRIV_SCHED_SETPOLICY when there is no distinction (mainly POSIX scheduler policies). Linux seems to support their own idle priority policies in sched_setscheduler(), but these do not translate to our POSIX API.

[kib]

Should we check for PRIV_SCHED_SETPARAM as well? The syscall sets both the policy and priority.

[olce]

The PRIV_SCHED_SETPOLICY and PRIV_SCHED_SET privileges are inconsistent with some other places and can be circumvented. Additionally, I don't think they serve any real security purposes (beyond what PRIV_SCHED_RTPRIO and PRIV_SCHED_IDPRIO can provide).

In the course of my priorities revamp project, they have simply been removed and replaced.

Given the extent of my changes in this area, I'm kindly asking that you refrain from major changes until I can commit them (which should take a few weeks more).

If fixing this is deemed urgent, this change can be committed as it is. But please abstain from further refinements and/or liaise with me first. Thanks.

[dev_submerge.ch]

In D43835#1000712, @olce wrote:

The PRIV_SCHED_SETPOLICY and PRIV_SCHED_SET privileges are inconsistent with some other places and can be circumvented. Additionally, I don't think they serve any real security purposes (beyond what PRIV_SCHED_RTPRIO and PRIV_SCHED_IDPRIO can provide).

I suppose PRIV_SCHED_SETPOLICY could be replaced by PRIV_SCHED_RTPRIO in current use cases - is that what you mean?

In the course of my priorities revamp project, they have simply been removed and replaced.

Didn't know about this project, is there an outline somewhere to read? As the maintainer of audio/jack I'd be particularly interested in changes affecting applications.

Given the extent of my changes in this area, I'm kindly asking that you refrain from major changes until I can commit them (which should take a few weeks more).

If fixing this is deemed urgent, this change can be committed as it is. But please abstain from further refinements and/or liaise with me first. Thanks.

I can't speak for @jbeich, but I think we have to work around this issue anyway, for the time being. Does your revamp project fix it so we can close this review, or should we just wait until your changes hit the tree?

[olce]

Hi Florian,

In D43835#1000809, @dev_submerge.ch wrote:

In D43835#1000712, @olce wrote:

I suppose PRIV_SCHED_SETPOLICY could be replaced by PRIV_SCHED_RTPRIO in current use cases - is that what you mean?

Yes and no. I mean, yes, you can as well interpret it like that, but my remark was more general, as I've rationalized privileges related to priorities (remove those that don't really make sense, ensure every entry point has the same privileges checked, etc.).

But I think the minimal working change for now is really to use PRIV_SCHED_SETPOLICY instead of PRIV_SCHED_SET. It is not correct in theory, but in practice good enough, since it still allows root (obviously) and works with mac_priority(4) for users in realtime.

Didn't know about this project, is there an outline somewhere to read? As the maintainer of audio/jack I'd be particularly interested in changes affecting applications.

Yes. I guess it's on me that I didn't communicate beyond the Foundation and committers up to now. I have been following very loosely what you have been doing in jack, virtual_oss uses, and sound in general (and also about security.bsd.unprivileged_idprio, which I'm considering undeprecating), so I was planning to contact you about that, since real-time is often used by multimedia applications, and sound in particular. A large part of the changes should be ready to be reviewed, but there is still a second round to do, and I first thought I'd finish that one before reaching out, but given your interest, I guess that time is now.

I've written an article about the project, to be presented at AsiaBSDCon 2024. Not sure if I can publish it right now. Could you send me an email (@FreeBSD.org) and I'll send it to you in reply? The changes part is a rough summary, and some changes are not mentioned, but this should give you a good idea of what's cooking.

I can't speak for @jbeich, but I think we have to work around this issue anyway, for the time being. Does your revamp project fix it so we can close this review, or should we just wait until your changes hit the tree?

Yes, this issue is fixed on my tree, as well as a whole lot of other problems with scheduling priorities in general. There are also some API behavior changes that we are going to discuss during the reviews. I'll make sure you're onboarded.

For this review, it depends on the urgency of the fix. If you want it very soon, I'm fine with the change here being committed. I just really would like to avoid more extensive changes, as all this code has already been rewritten in my tree and that would cause unpleasant rebases. The other option is just waiting for a few weeks. Really, it's up to you and jbeich@.

Thanks.

[dev_submerge.ch]

Thanks for the article, Olivier - now that I know the extent of your project, I suspect it won't be MFC'd?
If that's the case it may be worth to get this minimal fix in right now, and MFC it to STABLE. The earlier this issue is fixed in all supported releases, the less workarounds in ports.

[olce]

In D43835#1001005, @dev_submerge.ch wrote:

Thanks for the article, Olivier - now that I know the extent of your project, I suspect it won't be MFC'd?
If that's the case it may be worth to get this minimal fix in right now, and MFC it to STABLE. The earlier this issue is fixed in all supported releases, the less workarounds in ports.

The first round of changes, which includes fixes for privileges, do not touch what is considered the public KPI AFAIK so should be MFCable (and MFCed). The second round relies on some modifications of struct thread, and that I don't think can be MFCed.

That said, this change is really tiny, and as we are in the process of releasing 13.3 which well may be the last release from stable/13 (nothing official, just a risk assessment), to ease porters' job I think we should try to get this in 13.3 as well.

So going to commit this change as it is. Could you send me a patch prepared with git format-patch, else I can take the patch here and put your name and mail as the author myself (as you prefer)?

Thanks.

[olce]

In D43835#1001028, @olce wrote:

Could you send me a patch prepared with git format-patch, else I can take the patch here and put your name and mail as the author myself (as you prefer)?

Well, don't bother, I'm just grabbing your name and mail from pre-existing commits.

[imp]

In D43835#1001031, @olce wrote:

In D43835#1001028, @olce wrote:

Could you send me a patch prepared with git format-patch, else I can take the patch here and put your name and mail as the author myself (as you prefer)?

Well, don't bother, I'm just grabbing your name and mail from pre-existing commits.

Git arc parch -c D43835 likely works and is less hassle

[olce]

In D43835#1001048, @imp wrote:

Git arc parch -c D43835 likely works and is less hassle

Thanks.