From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752522AbdLUNzy (ORCPT ); Thu, 21 Dec 2017 08:55:54 -0500 Received: from mx2.suse.de ([195.135.220.15]:46193 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751766AbdLUNzt (ORCPT ); Thu, 21 Dec 2017 08:55:49 -0500 Date: Thu, 21 Dec 2017 14:55:47 +0100 (CET) From: Miroslav Benes To: Petr Mladek cc: Josh Poimboeuf , 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 In-Reply-To: <20171221133004.lilrqfiu3btb74lj@pathway.suse.cz> Message-ID: References: <20171208172523.12150-1-mbenes@suse.cz> <20171208172523.12150-2-mbenes@suse.cz> <20171220143512.7uawixpxkxre6n7i@pathway.suse.cz> <20171220170937.2wow5fkxoza2cf6v@treble> <20171221133004.lilrqfiu3btb74lj@pathway.suse.cz> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 21 Dec 2017, Petr Mladek wrote: > On Wed 2017-12-20 11:09:37, Josh Poimboeuf wrote: > > On Wed, Dec 20, 2017 at 03:35:12PM +0100, Petr Mladek wrote: > > > 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. > > > > > > > > > > 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. > > > > > > 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. > > > > "Using the force" is a nuclear option. User beware. If you use it (or > > recommend that others use it), be prepared for the consequences. That > > means anticipating how forcing this patch might affect future patches, > > and planning accordingly. > > > > >From a livepatch code standpoint, let's avoid adding complexity (or > > limitations) where none are needed. I think all we need to do is > > permanently disable patch module unloading when somebody forces a patch, > > which we already do. Otherwise the user is on their own. I agree with Josh here. If there is a problem with a patch module, the recommended action is to simply cancel its transition (by writing 0 to enabled). If there are serious reasons to apply the patch, there is force as the last resort. In that case the user should probably plan for reboot into an updated kernel and not to plan to apply more live patches. > This looks like a rather weak protection against nuclear diseases ;-) > > If we want to keep it simple and safe, we should either print > a big fact warning about this danger when the option is used. > Or we should allow to load new patches only with yet another > force flag. Having said the above, I'm not against to update the warning and documentation. I would not introduce another force flag to deal with it. Thanks, Miroslav