From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1EF81C2BB85 for ; Thu, 16 Apr 2020 09:28:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0526A206E9 for ; Thu, 16 Apr 2020 09:28:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2392206AbgDPJ2b (ORCPT ); Thu, 16 Apr 2020 05:28:31 -0400 Received: from mx2.suse.de ([195.135.220.15]:59948 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2392188AbgDPJ22 (ORCPT ); Thu, 16 Apr 2020 05:28:28 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E9FEBAD0E; Thu, 16 Apr 2020 09:28:25 +0000 (UTC) Date: Thu, 16 Apr 2020 11:28:25 +0200 (CEST) From: Miroslav Benes To: Josh Poimboeuf cc: Jessica Yu , live-patching@vger.kernel.org, linux-kernel@vger.kernel.org, Peter Zijlstra Subject: Re: [PATCH 6/7] livepatch: Remove module_disable_ro() usage In-Reply-To: <20200415163303.ubdnza6okg4h3e5a@treble> Message-ID: References: <9f0d8229bbe79d8c13c091ed70c41d49caf598f2.1586881704.git.jpoimboe@redhat.com> <20200415150216.GA6164@linux-8ccs.fritz.box> <20200415163303.ubdnza6okg4h3e5a@treble> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: live-patching-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: live-patching@vger.kernel.org On Wed, 15 Apr 2020, Josh Poimboeuf wrote: > On Wed, Apr 15, 2020 at 05:02:16PM +0200, Jessica Yu wrote: > > +++ Josh Poimboeuf [14/04/20 11:28 -0500]: > > > With arch_klp_init_object_loaded() gone, and apply_relocate_add() now > > > using text_poke(), livepatch no longer needs to use module_disable_ro(). > > > > > > The text_mutex usage can also be removed -- its purpose was to protect > > > against module permission change races. > > > > > > Signed-off-by: Josh Poimboeuf > > > --- > > > kernel/livepatch/core.c | 8 -------- > > > 1 file changed, 8 deletions(-) > > > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > > index 817676caddee..3a88639b3326 100644 > > > --- a/kernel/livepatch/core.c > > > +++ b/kernel/livepatch/core.c > > > @@ -767,10 +767,6 @@ static int klp_init_object_loaded(struct klp_patch *patch, > > > struct klp_modinfo *info = patch->mod->klp_info; > > > > > > if (klp_is_module(obj)) { > > > - > > > - mutex_lock(&text_mutex); > > > - module_disable_ro(patch->mod); > > > - > > > > Don't you still need the text_mutex to use text_poke() though? > > (Through klp_write_relocations -> apply_relocate_add -> text_poke) > > At least, I see this assertion there: > > > > void *text_poke(void *addr, const void *opcode, size_t len) > > { > > lockdep_assert_held(&text_mutex); > > > > return __text_poke(addr, opcode, len); > > } > > Hm, guess I should have tested with lockdep ;-) :) If I remember correctly, text_mutex must be held whenever text is modified to prevent race due to the modification. It is not only about permission changes even though it was how it manifested itself in our case. Miroslav