Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()

From: Joe Lawrence <joe.lawrence@redhat.com>
To: Petr Mladek <pmladek@suse.com>, Miroslav Benes <mbenes@suse.cz>
Cc: Will Deacon <will@kernel.org>,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	akpm@linux-foundation.org,
	"K . Prasad" <prasad@linux.vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Quentin Perret <qperret@google.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	live-patching@vger.kernel.org
Subject: Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
Date: Tue, 25 Feb 2020 10:00:45 -0500	[thread overview]
Message-ID: <943e7093-2862-53c6-b7f4-96c7d65789b9@redhat.com> (raw)
In-Reply-To: <20200225121125.psvuz6e7coa77vxe@pathway.suse.cz>

On 2/25/20 7:11 AM, Petr Mladek wrote:
> On Tue 2020-02-25 11:05:39, Miroslav Benes wrote:
>> CC live-patching ML, because this could affect many of its users...
>>
>> On Fri, 21 Feb 2020, Will Deacon wrote:
>>
>>> Hi folks,
>>>
>>> Despite having just a single modular in-tree user that I could spot,
>>> kallsyms_lookup_name() is exported to modules and provides a mechanism
>>> for out-of-tree modules to access and invoke arbitrary, non-exported
>>> kernel symbols when kallsyms is enabled.
> 
> Just to explain how this affects livepatching users.
> 
> Livepatch is a module that inludes fixed copies of functions that
> are buggy in the running kernel. These functions often
> call functions or access variables that were defined static in
> the original source code. There are two ways how this is currently
> solved.
> 
> Some livepatch authors use kallsyms_lookup_name() to locate the
> non-exported symbols in the running kernel and then use these
> address in the fixed code.
> 

FWIW, kallsyms was historically used by the out-of-tree kpatch support 
module to resolve external symbols as well as call set_memory_r{w,o}() 
API.  All of that support code has been merged upstream, so modern 
kpatch modules* no longer leverage kallsyms by default.

* That said, there are still some users who still use the deprecated 
support module with newer kernels, but that is not officially supported 
by the project.

> Another possibility is to used special relocation sections,
> see Documentation/livepatch/module-elf-format.rst
> 
> The problem with the special relocations sections is that the support
> to generate them is not ready yet. The main piece would klp-convert
> tool. Its development is pretty slow. The last version can be
> found at
> https://lkml.kernel.org/r/20190509143859.9050-1-joe.lawrence@redhat.com
> 
> I am not sure if this use case is enough to keep the symbols exported.
> Anyway, there are currently some out-of-tree users.
> 

Another (temporary?) klp-relocation issue is that binutils has limited 
support for them as currently implemented:

   https://sourceware.org/ml/binutils/2020-02/msg00317.html

For example, try running strip or objcopy on a .ko that includes them 
and you may find surprising results :(

As far as the klp-convert patchset goes, I forget whether or not we tied 
its use case to source-based livepatch creation.  If kallsyms goes 
unexported, perhaps it finds more immediate users.

However since klp-convert provides nearly the same functionality as 
kallsyms, i.e. both can be used to circumvent symbol export licensing -- 
one could make similar arguments against its inclusion.

If there is renewed (or greater, to be more accurate) interest in the 
klp-convert patchset, we can dust it off and see what's left.  AFAIK it 
was blocked on arch-specific klp-relocations and whether per-object 
livepatch modules would remove that requirement.

-- Joe