From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752480AbdLUNaO (ORCPT ); Thu, 21 Dec 2017 08:30:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:43324 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752311AbdLUNaG (ORCPT ); Thu, 21 Dec 2017 08:30:06 -0500 Date: Thu, 21 Dec 2017 14:30:04 +0100 From: Petr Mladek To: Josh Poimboeuf Cc: Miroslav Benes , 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: <20171221133004.lilrqfiu3btb74lj@pathway.suse.cz> References: <20171208172523.12150-1-mbenes@suse.cz> <20171208172523.12150-2-mbenes@suse.cz> <20171220143512.7uawixpxkxre6n7i@pathway.suse.cz> <20171220170937.2wow5fkxoza2cf6v@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171220170937.2wow5fkxoza2cf6v@treble> 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 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. 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. Anyway, I agree that we should keep it simple. The fact is that the immediate flag removal makes the code better readable. Best Regards, Petr