From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755455AbdLTOfP (ORCPT ); Wed, 20 Dec 2017 09:35:15 -0500 Received: from mx2.suse.de ([195.135.220.15]:57884 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753868AbdLTOfN (ORCPT ); Wed, 20 Dec 2017 09:35:13 -0500 Date: Wed, 20 Dec 2017 15:35:12 +0100 From: Petr Mladek To: Miroslav Benes Cc: jpoimboe@redhat.com, jeyu@kernel.org, jikos@kernel.org, linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, jbaron@akamai.com Subject: Re: [PATCH 1/2] livepatch: Remove immediate feature Message-ID: <20171220143512.7uawixpxkxre6n7i@pathway.suse.cz> References: <20171208172523.12150-1-mbenes@suse.cz> <20171208172523.12150-2-mbenes@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171208172523.12150-2-mbenes@suse.cz> User-Agent: NeoMutt/20170421 (1.8.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 2017-12-08 18:25:22, Miroslav Benes wrote: > immediate flag has been used to disable per-task consistency and patch > all tasks immediately. It could be useful if the patch doesn't change any > function or data semantics. > > However, it causes problems on its own. The consistency problem is > currently broken with respect to immediate patches. > > func a > patches 1i > 2i > 3 > > When the patch 3 is applied, only 2i function is checked (by stack > checking facility). There might be a task sleeping in 1i though. Such > task is migrated to 3, because we do not check 1i in > klp_check_stack_func() at all. > > Coming atomic replace feature would be easier to implement and more > reliable without immediate. > > Moreover, the fake signal and force feature give us almost the same > benefits and the user can decide to use them in problematic situations > (while immediate needs to be set before the patch is applied). It is > also more isolated in terms of code. > > Thus, remove immediate feature completely and save us from the problems. Sigh, the force feature actually have the same problem. We would use it when a process never has a reliable stack or when it is endlessly sleeping in a function that might have been patched immediately. The documentation about the force feature says that the user should consult the patch provider before using the flag. The provider would check that it is really safe in the given situation and eventually allow to use the force. But what about any future livepatches? If the force flag was safe for a given livepatch/process, it does not mean that it would be safe for the next one. The process might still be sleeping on the original function or on one lower in the stack. I see two possibilities. We could either refuse loading new livepatches after using the force flag. Or we would need to check all variants of the function "a" that might still be in use. I think that we might want to check the stack correctly. Note that we need to take care also about livepatches that were disabled. They are usually removed from func->stack_node. We might need to maintain separate stack where we would put all variants of the function that might be in use when using the immediate or force flag. I am not sure if we still want to remove the immediate flag then. Best Regards, Petr