live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
       [not found] <20200221114404.14641-1-will@kernel.org>
@ 2020-02-25 10:05 ` Miroslav Benes
  2020-02-25 12:11   ` Petr Mladek
  0 siblings, 1 reply; 5+ messages in thread
From: Miroslav Benes @ 2020-02-25 10:05 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-kernel, kernel-team, akpm, K . Prasad, Thomas Gleixner,
	Greg Kroah-Hartman, Frederic Weisbecker, Christoph Hellwig,
	Quentin Perret, Alexei Starovoitov, Masami Hiramatsu,
	live-patching

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.
> 
> This patch series fixes up that one user and unexports the symbol along
> with kallsyms_on_each_symbol(), since that could also be abused in a
> similar manner.
> 
> Cheers,
> 
> Will
> 
> Cc: K.Prasad <prasad@linux.vnet.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Quentin Perret <qperret@google.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> 
> --->8
> 
> Will Deacon (3):
>   samples/hw_breakpoint: Drop HW_BREAKPOINT_R when reporting writes
>   samples/hw_breakpoint: Drop use of kallsyms_lookup_name()
>   kallsyms: Unexport kallsyms_lookup_name() and
>     kallsyms_on_each_symbol()
> 
>  kernel/kallsyms.c                       |  2 --
>  samples/hw_breakpoint/data_breakpoint.c | 11 ++++++++---
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> -- 
> 2.25.0.265.gbab2e86ba0-goog
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
  2020-02-25 10:05 ` [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol() Miroslav Benes
@ 2020-02-25 12:11   ` Petr Mladek
  2020-02-25 15:00     ` Joe Lawrence
  0 siblings, 1 reply; 5+ messages in thread
From: Petr Mladek @ 2020-02-25 12:11 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Will Deacon, linux-kernel, kernel-team, akpm, K . Prasad,
	Thomas Gleixner, Greg Kroah-Hartman, Frederic Weisbecker,
	Christoph Hellwig, Quentin Perret, Alexei Starovoitov,
	Masami Hiramatsu, live-patching

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.

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.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
  2020-02-25 12:11   ` Petr Mladek
@ 2020-02-25 15:00     ` Joe Lawrence
  2020-02-25 18:01       ` Miroslav Benes
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Lawrence @ 2020-02-25 15:00 UTC (permalink / raw)
  To: Petr Mladek, Miroslav Benes
  Cc: Will Deacon, linux-kernel, kernel-team, akpm, K . Prasad,
	Thomas Gleixner, Greg Kroah-Hartman, Frederic Weisbecker,
	Christoph Hellwig, Quentin Perret, Alexei Starovoitov,
	Masami Hiramatsu, live-patching

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
  2020-02-25 15:00     ` Joe Lawrence
@ 2020-02-25 18:01       ` Miroslav Benes
  2020-02-26 14:16         ` Joe Lawrence
  0 siblings, 1 reply; 5+ messages in thread
From: Miroslav Benes @ 2020-02-25 18:01 UTC (permalink / raw)
  To: Joe Lawrence
  Cc: Petr Mladek, Will Deacon, linux-kernel, kernel-team, akpm,
	K . Prasad, Thomas Gleixner, Greg Kroah-Hartman,
	Frederic Weisbecker, Christoph Hellwig, Quentin Perret,
	Alexei Starovoitov, Masami Hiramatsu, live-patching

[-- Attachment #1: Type: text/plain, Size: 3764 bytes --]

On Tue, 25 Feb 2020, Joe Lawrence wrote:

> 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.

Good. Quick grep through the sources gave me a couple of hits, so I was 
not sure.
 
> * 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.

In a way yes, but as Masami described elsewhere in the thread there are 
more convenient ways to circumvent it even now. Not as convenient as 
kallsyms, of course. 
 
> 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.

Yes, I think it is on standby now. I thought about it while walking 
through Petr's module split patch set and it seemed to me that klp-convert 
could be made much much simpler on top of that. So we should start there.

Anyway, as far as Will's patch set is concerned, there is no real obstacle 
on our side, is there?

Alexei mentioned ksplice from git history, but no one cares about ksplice 
in upstream now, I would say.

Thanks
Miroslav

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
  2020-02-25 18:01       ` Miroslav Benes
@ 2020-02-26 14:16         ` Joe Lawrence
  0 siblings, 0 replies; 5+ messages in thread
From: Joe Lawrence @ 2020-02-26 14:16 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Petr Mladek, Will Deacon, linux-kernel, kernel-team, akpm,
	K . Prasad, Thomas Gleixner, Greg Kroah-Hartman,
	Frederic Weisbecker, Christoph Hellwig, Quentin Perret,
	Alexei Starovoitov, Masami Hiramatsu, live-patching

On 2/25/20 1:01 PM, Miroslav Benes wrote:
> Anyway, as far as Will's patch set is concerned, there is no real obstacle
> on our side, is there?
> 

It places greater importance on getting the klp-relocation parts 
correct, but assuming is is/will be then I think we're good.

-- Joe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-26 14:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200221114404.14641-1-will@kernel.org>
2020-02-25 10:05 ` [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol() Miroslav Benes
2020-02-25 12:11   ` Petr Mladek
2020-02-25 15:00     ` Joe Lawrence
2020-02-25 18:01       ` Miroslav Benes
2020-02-26 14:16         ` Joe Lawrence

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).