Page MenuHomeFreeBSD

Add cap_sysctl() and cap_sysctlnametomib().
ClosedPublic

Authored by markj on Nov 6 2018, 12:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 20 2023, 4:48 AM
Unknown Object (File)
Nov 15 2023, 9:40 AM
Unknown Object (File)
Nov 11 2023, 9:19 PM
Unknown Object (File)
Nov 8 2023, 4:48 PM
Unknown Object (File)
Oct 14 2023, 8:39 AM
Unknown Object (File)
Oct 8 2023, 8:15 PM
Unknown Object (File)
Oct 7 2023, 3:33 PM
Unknown Object (File)
Aug 30 2023, 6:33 AM
Subscribers

Details

Summary

Most applications these days use sysctlbyname(), but some still use sysctl()
for a few reasons:

  • legacy/BSD-portable code
  • performance (sysctlbyname requires multiple system calls)
  • per-resource sysctl trees (e.g., CTL_KERN.KERN_PROC.KERN_PROC_PID)

To make libcap_sysctl a better drop-in replacement for the
non-capsicumized functions, I added cap_sysctl(3) and cap_sysctlnametomib().
I also revised the limit interface to accommodate the new interfaces, and to
avoid direct manipulation of nvlists.

cap_sysctl(3) and cap_sysctlnametomib(3) are subject to the same limits as
named sysctls. Limits can be specified by name or by MIB. When a limit is
specified by name, we automatically resolve the name to a MIB and update
the limit. This is made somewhat awkward by the fact that the "limits" nvlist
in the service command handler is const; that is, we cannot modify the service
limits in the sysctlnametomib command handler.

The old code used a very simple format for limit nvlists: the sysctl name was a key
and the allowed operations was stored as a number value. Obviously this doesn't
work for MIBs, so I extended the nvlist format somewhat. Now, the set of limits is
an nvlist of nvlists. Each sub-nvlist contains one or both of "name" and "mib", and
also contains the allowed operations ("operation"). See the updated man page for
the new limit interface.

While testing I found that cap_sysctl is much slower than direct system calls.
A loop which just reads a sysctl shows a 15x increase in runtime. This overhead
is a combination of IPC overhead, nvlist construction, and nvlist serialization; I
did not test with any limits set, but that would have made it worse. I
suspect that the only true solution is to provide a "sysctlfd" abstraction which
allows applications to reference subtrees of the sysctl tree. However, cap_sysctl
is still useful for sandboxing existing applications without requiring non-trivial changes.

Test Plan

Regression tests.

I capsicumized powerd(8), which makes use of sysctl(3) and sysctlnametomib(3).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj edited the test plan for this revision. (Show Details)
markj added reviewers: capsicum, oshogbo.
markj marked an inline comment as done.Nov 6 2018, 1:09 AM
markj added inline comments.
lib/libcasper/services/cap_sysctl/cap_sysctl.c
165 ↗(On Diff #50049)

This is a bug fix which I'll commit separately. If a consumer called sysctlbyname with oldp == NULL, to get the size of the sysctl variable, "operation" would be left equal to 0, and sysctl_valid() would fail. So, the cap_sysctl interface could not be used to get a sysctl variable's size.

oshogbo requested changes to this revision.Nov 12 2018, 3:28 PM
oshogbo added inline comments.
lib/libcasper/services/cap_sysctl/Makefile
9 ↗(On Diff #50049)

Missing ObsoleteFile?

29 ↗(On Diff #50049)

Missing the cap_sysctlnametomib ?

lib/libcasper/services/cap_sysctl/cap_sysctl.c
58 ↗(On Diff #50049)

I thing there should be a typedef in '.h' I don't like that we return 'void *'.

70 ↗(On Diff #50049)

Do we care what channel this limits are?

73 ↗(On Diff #50049)

It can override the errno right?

81 ↗(On Diff #50049)

Why not to do a typdef struct cap_sysctl_limit in header?

94 ↗(On Diff #50049)

Why we destroing limits here?

95 ↗(On Diff #50049)

Isn't that weird that we free structure from user?

103 ↗(On Diff #50049)

Do we care about resolving those mibs here?
Can't casper do that in more safe way?

107 ↗(On Diff #50049)

You want to do here move.
Otherwise you are leaking structure.

136 ↗(On Diff #50049)

I can't use multiple time those some limits for different instances.

362 ↗(On Diff #50049)

We can use dnvlist here

This revision now requires changes to proceed.Nov 12 2018, 3:28 PM
markj marked 15 inline comments as done.Nov 12 2018, 7:39 PM
markj added inline comments.
lib/libcasper/services/cap_sysctl/Makefile
29 ↗(On Diff #50049)

Also the limit functions.

lib/libcasper/services/cap_sysctl/cap_sysctl.c
58 ↗(On Diff #50049)

I will typedef struct cap_sysctl_limit cap_sysctl_limit_t in the header.

70 ↗(On Diff #50049)

This is only used to perform the name->mib resolution discussed below.

81 ↗(On Diff #50049)

No good reason, this is fixed in my version now.

94 ↗(On Diff #50049)

Mostly so that I don't have to provide a separate cap_sysctl_limit_free() function. I could change it though.

95 ↗(On Diff #50049)

Kind of, but it is consistent with libcasper. cap_limit_set() always frees the input nvlist, for example.

103 ↗(On Diff #50049)

I mentioned this in the review description. Where else can we do the resolution? libcasper forbids modification of the limit nvlist from the service routines by marking them const. The only way I can see to change them is to perform resolution externally.

136 ↗(On Diff #50049)

The same is true of the base cap_limit_set() interface.

markj marked 3 inline comments as done.

Address some of the feedback. I left replies for the rest.

Really sorry for keep you waiting.

This revision is now accepted and ready to land.Mar 24 2019, 1:28 AM
This revision was automatically updated to reflect the committed changes.