Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755424AbbG1Mwu (ORCPT ); Tue, 28 Jul 2015 08:52:50 -0400 Received: from casper.infradead.org ([85.118.1.10]:43878 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbbG1Mwt (ORCPT ); Tue, 28 Jul 2015 08:52:49 -0400 Date: Tue, 28 Jul 2015 14:52:33 +0200 From: Peter Zijlstra To: Rusty Russell Cc: He Kuang , wangnan0@huawei.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] module: Fix missing to hold module_mutex lock in module_kallsyms_lookup_name Message-ID: <20150728125233.GA19282@twins.programming.kicks-ass.net> References: <1437991286-101906-1-git-send-email-hekuang@huawei.com> <878ua1p1g8.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <878ua1p1g8.fsf@rustcorp.com.au> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1780 Lines: 52 On Tue, Jul 28, 2015 at 10:01:35AM +0930, Rusty Russell wrote: > Peter? > module: weaken locking assertion for oops path. > > We don't actually hold the module_mutex when calling find_module_all > from module_kallsyms_lookup_name: that's because it's used by the oops > code and we don't want to deadlock. > > However, access to the list read-only is safe if preempt is disabled, > so we can weaken the assertion. Keep a strong version for external > callers though. > > Fixes: 0be964be0d45 ("module: Sanitize RCU usage and locking") > Reported-by: He Kuang > Signed-off-by: Rusty Russell > > diff --git a/kernel/module.c b/kernel/module.c > index 4d2b82e610e2..b86b7bf1be38 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -602,13 +602,16 @@ const struct kernel_symbol *find_symbol(const char *name, > } > EXPORT_SYMBOL_GPL(find_symbol); > > -/* Search for module by name: must hold module_mutex. */ > +/* > + * Search for module by name: must hold module_mutex (or preempt disabled > + * for read-only access). > + */ > static struct module *find_module_all(const char *name, size_t len, > bool even_unformed) > { > struct module *mod; > > - module_assert_mutex(); > + module_assert_mutex_or_preempt(); Yeah, that should be fine indeed, I went by that comment you just expanded. The operation itself does indeed not modify data at all, so the preempt_disable is perfectly adequate. Thanks! Acked-by: Peter Zijlstra (Intel) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/