Re: [PATCH v8 1/2] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options

[Kees Cook <keescook@chromium.org>]

On Wed, Jun 26, 2019 at 02:15:49PM -0400, Qian Cai wrote:
> On Wed, 2019-06-26 at 14:19 +0200, Alexander Potapenko wrote:
> > Both init_on_alloc and init_on_free default to zero, but those defaults
> > can be overridden with CONFIG_INIT_ON_ALLOC_DEFAULT_ON and
> > CONFIG_INIT_ON_FREE_DEFAULT_ON.
> > [...]
> > +static int __init early_init_on_alloc(char *buf)
> > +{
> > +	int ret;
> > +	bool bool_result;
> > +
> > +	if (!buf)
> > +		return -EINVAL;
> > +	ret = kstrtobool(buf, &bool_result);
> > +	if (bool_result)
> > +		static_branch_enable(&init_on_alloc);
> > +	else
> > +		static_branch_disable(&init_on_alloc);
> > +	return ret;
> > +}
> > +early_param("init_on_alloc", early_init_on_alloc);
> 
> Do those really necessary need to be static keys?
> 
> Adding either init_on_free=0 or init_on_alloc=0 to the kernel cmdline will
> generate a warning with kernels built with clang.
> 
> [    0.000000] static_key_disable(): static key 'init_on_free+0x0/0x4' used
> before call to jump_label_init()
> [    0.000000] WARNING: CPU: 0 PID: 0 at ./include/linux/jump_label.h:317
> early_init_on_free+0x1c0/0x200
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.2.0-rc6-next-20190626+
> #9
> [    0.000000] pstate: 60000089 (nZCv daIf -PAN -UAO)

I think the issue here is that arm64 doesn't initialize static keys
early enough.

init/main.c has the general case:

asmlinkage __visible void __init start_kernel(void)
{
        ...
        setup_arch(&command_line);
        ...
        smp_prepare_boot_cpu();
        ...
        /* parameters may set static keys */
        jump_label_init();
        parse_early_param();
        ...
}

however, x86 does even earlier early params in setup_arch():

void __init setup_arch(char **cmdline_p)
{
        ...
        jump_label_init();
        ...
        parse_early_param();
        ...
}

arm64 does similar very early early params in setup_arch()[1] too,
but not jump_label_init() which is too late in smp_prepare_boot_cpu():

void __init setup_arch(char **cmdline_p)
{
        ...
        parse_early_param();
        ...
}

void __init smp_prepare_boot_cpu(void)
{
        ...
        jump_label_init();
        ...
}

I can send a patch to fix this...

-Kees

[1] since efd9e03facd07 ("arm64: Use static keys for CPU features")

> [    0.000000] pc : early_init_on_free+0x1c0/0x200
> [    0.000000] lr : early_init_on_free+0x1c0/0x200
> [    0.000000] sp : ffff100012c07df0
> [    0.000000] x29: ffff100012c07e20 x28: ffff1000110a01ec 
> [    0.000000] x27: 000000000000005f x26: ffff100011716cd0 
> [    0.000000] x25: ffff100010d36166 x24: ffff100010d3615d 
> [    0.000000] x23: ffff100010d364b5 x22: ffff1000117164a0 
> [    0.000000] x21: 0000000000000000 x20: 0000000000000000 
> [    0.000000] x19: 0000000000000000 x18: 000000000000002e 
> [    0.000000] x17: 000000000000000f x16: 0000000000000040 
> [    0.000000] x15: 0000000000000000 x14: 6c61632065726f66 
> [    0.000000] x13: 6562206465737520 x12: 273478302f307830 
> [    0.000000] x11: 0000000000000000 x10: 0000000000000000 
> [    0.000000] x9 : 0000000000000000 x8 : 0000000000000000 
> [    0.000000] x7 : 6d756a206f74206c x6 : ffff100014426625 
> [    0.000000] x5 : ffff100012c07b28 x4 : 0000000000000007 
> [    0.000000] x3 : ffff1000101aadf4 x2 : 0000000000000001 
> [    0.000000] x1 : 0000000000000001 x0 : 000000000000005d 
> [    0.000000] Call trace:
> [    0.000000]  early_init_on_free+0x1c0/0x200
> [    0.000000]  do_early_param+0xd0/0x104
> [    0.000000]  parse_args+0x1f0/0x524
> [    0.000000]  parse_early_param+0x70/0x8c
> [    0.000000]  setup_arch+0xa8/0x268
> [    0.000000]  start_kernel+0x80/0x560
> 

-- 
Kees Cook