linux-kernel - Re: [PATCH v2 1/3] livepatch: Add force sysfs attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170816131552.GA601@pathway.suse.cz>
Date:   Wed, 16 Aug 2017 15:15:52 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Miroslav Benes <mbenes@...e.cz>
Cc:     jpoimboe@...hat.com, jeyu@...nel.org, jikos@...nel.org,
        lpechacek@...e.cz, live-patching@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] livepatch: Add force sysfs attribute

On Thu 2017-08-10 12:48:13, Miroslav Benes wrote:
> Add read-write force attribute to livepatch sysfs infrastructure. We can
> use it later to force couple of events during a live patching process.
> Be it a sending of a fake signal or forcing of the tasks' successful
> conversion.
> 
> It does not make sense to use the force facility when there is no
> transaction running (although there is no harm doing that). Therefore we
> limit it only to situations when klp_transition_patch variable is set.
> Normally, klp_mutex lock should be acquired, because the variable is
> shared. However that would hold the action back unnecessarily because of
> waiting for the lock, so we omit the lock here. The resulting race
> window is harmless (using force when there is no transaction running).
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index b9628e43c78f..79022b7eca2c 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -954,6 +990,13 @@ static int __init klp_init(void)
>  	if (!klp_root_kobj)
>  		return -ENOMEM;
>  
> +	ret = sysfs_create_group(klp_root_kobj, &klp_sysfs_group);
> +	if (ret) {
> +		pr_err("cannot create livepatch attributes in sysfs\n");
> +		kobject_put(klp_root_kobj);

We need to set klp_root_kobj = NULL here. Or we need to set the global
klp_root_kobj only when the attributes are created. Otherwise,
klp_initialized() would return true and registering a patch would
push the system out of a safe road.

Note that this actually opens a small race window when the livepatching
core pretends to be initialized even when the initialization still
might fail. It is rather theoretical but it would be nice to avoid
it if it can be done an easy way, e.g. by setting klp_root_kobj later.

Best Regards,
Petr