Re: [RFC] kernel/sysctl: support setting sysctl parameters from kernel command line

On 3/17/20 10:29 PM, Kees Cook wrote:

On Tue, Mar 17, 2020 at 02:21:05PM +0100, Vlastimil Babka wrote:

quoted

A recently proposed patch to add vm_swappiness command line parameter in
addition to existing sysctl [1] made me wonder why we don't have a general
support for passing sysctl parameters via command line. Googling found only
somebody else wondering the same [2], but I haven't found any prior discussion
with reasons why not to do this.

I'd like to see stuff like this (as you say, you've found some
redundancies here which could be cleaned up a bit). I think the reason
it hasn't happened before is that the answers have mostly revolved
around "just set it in your initramfs". :P

quoted

[...]
Hence, this patch adds a new parse_args() pass that looks for parameters
prefixed by 'sysctl.' and searches for them in the sysctl ctl_tables. When
found, the respective proc handler is invoked. The search is just a naive
linear one, to avoid using the whole procfs layer. It should be acceptable,
as the cost depends on number of sysctl. parameters passed.

I think this needs reconsidering: this RFC only searches 1 level deep,
but sysctls are a tree. For example:

Yes, that was an oversight, easily fixed.

kernel.yama.ptrace_scope
mm.transparent_hugepage.enabled

Hm, transparent_hugepage is in sysfs (/sys/kernel/mm), but not sysctl, at least
in my case the sysctl tool doesn't list it. Yours does? Yay for consistency.

net.ipv4.conf.default.rp_filter
...etc

If this goes in, it'll need to do full traversal.

Right.

quoted

The main limitation of avoiding the procfs layer is however that sysctls
dynamically registered by register_sysctl_table() or register_sysctl_paths()
cannot be set by this method.

Correct. And I like what you've done in the code: announce any unhandled
sysctls.

quoted

The processing is hooked right before the init process is loaded, as some
handlers might be more complicated than simple setters and might need some
subsystems to be initialized. At the moment the init process can be started and
eventually execute a process writing to /proc/sys/ then it should be also fine
to do that from the kernel.

I agree about placement.

quoted

[1] https://lore.kernel.org/linux-doc/BL0PR02MB560167492CA4094C91589930E9FC0@BL0PR02MB5601.namprd02.prod.outlook.com/ (local)
[2] https://unix.stackexchange.com/questions/558802/how-to-set-sysctl-using-kernel-command-line-parameter

Signed-off-by: Vlastimil Babka <redacted>
---
Hi,

this is an early RFC so I can get feedback whether to pursue this idea further,
before trying the more complicated stuff with dynamically registered sysctls.
For those I have some unanswered questions:
- Support them at all?

Maybe? It seems excessive for the initial version.

OK

quoted

- Do so by an internal procfs mount again, that was removed by 61a47c1ad3a4 ?
  Or try to keep it simple.

I think you can walk the registered sysctl structures themselves, yes?

I should be able to, yeah.

quoted

- If sysctls are dynamically registered at module load, process the command
  line sysctl arguments again? - this would be rather complicated I guess.

If it does get supported, perhaps saving them somewhere for
register_sysctl_table() to walk when it gets called?

I like the idea if just for having to build less boiler plate for
supporting things that I've had to plumb to both boot_params and sysctl.
:)

Thanks, I will pursue the idea further then :)

Vlastimil