All of
 help / color / mirror / Atom feed
From: Will Deacon <>
To: Masami Hiramatsu <>
	"K . Prasad" <>,
	Thomas Gleixner <>,
	Greg Kroah-Hartman <>,
	Frederic Weisbecker <>,
	Christoph Hellwig <>,
	Quentin Perret <>,
	Alexei Starovoitov <>
Subject: Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()
Date: Fri, 21 Feb 2020 14:48:54 +0000	[thread overview]
Message-ID: <20200221144853.GA18153@willie-the-truck> (raw)
In-Reply-To: <>

Hi Masami,

On Fri, Feb 21, 2020 at 11:27:46PM +0900, Masami Hiramatsu wrote:
> On Fri, 21 Feb 2020 11:44:01 +0000
> Will Deacon <> wrote:
> > 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.
> What kind of issue would you like to fix with this?

I would like to avoid out-of-tree modules being easily able to call
functions that are not exported. kallsyms_lookup_name() makes this
trivial to the point that there is very little incentive to rework these
modules to either use upstream interfaces correctly or propose functionality
which may be otherwise missing upstream. Both of these latter solutions
would be pre-requisites to upstreaming these modules, and the current state
of things actively discourages that approach.

The background here is that we are aiming for Android devices to be able
to use a generic binary kernel image closely following upstream, with
any vendor extensions coming in as kernel modules. In this case, we
(Google) end up maintaining the binary module ABI within the scope of a
single LTS kernel. Monitoring and managing the ABI surface is not feasible
if it effectively includes all data and functions via kallsyms_lookup_name().
Of course, we could just carry this patch in the Android kernel tree,
but we're aiming to carry as little as possible (ideally nothing) and
I think it's a sensible change in its own right. I'm surprised you object
to it, in all honesty.

Now, you could turn around and say "that's not upstream's problem", but
it still seems highly undesirable to me to have an upstream bypass for
exported symbols that isn't even used by upstream modules. It's ripe for
abuse and encourages people to work outside of the upstream tree. The
usual rule is that we don't export symbols without a user in the tree
and that seems especially relevant in this case.

> There are many ways to find (estimate) symbol address, especially, if
> the programmer already has the symbol map, it is *very* easy to find
> the target symbol address even from one exported symbol (the distance
> of 2 symbols doesn't change.) If not, they can use kprobes to find
> their required symbol address. If they have a time, they can use
> snprintf("%pF") to search symbol.

I would say that both of these are inconvenient enough that the developer
would think twice before considering to use them in production.

> So, for me, this series just make it hard for casual developers (but
> maybe they will find the answer on any technical Q&A site soon).

Which casual developers? I don't understand who you're referring to here.
Do you have a specific example in mind?

> Hmm, are there other good way to detect such bad-manner out-of-tree
> module and reject them? What about decoding them and monitor their
> all call instructions? 

That sounds like using a sledge hammer to crack a nut to me.


  reply	other threads:[~2020-02-21 14:49 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-21 11:44 Will Deacon
2020-02-21 11:44 ` [PATCH 1/3] samples/hw_breakpoint: Drop HW_BREAKPOINT_R when reporting writes Will Deacon
2020-02-21 14:12   ` Christoph Hellwig
2020-02-21 11:44 ` [PATCH 2/3] samples/hw_breakpoint: Drop use of kallsyms_lookup_name() Will Deacon
2020-02-21 14:13   ` Christoph Hellwig
2020-02-21 14:20     ` Will Deacon
2020-02-21 11:44 ` [PATCH 3/3] kallsyms: Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol() Will Deacon
2020-02-21 11:53   ` Greg Kroah-Hartman
2020-02-21 14:14   ` Christoph Hellwig
2020-02-21 15:11   ` Alexei Starovoitov
2020-02-21 14:27 ` [PATCH 0/3] " Masami Hiramatsu
2020-02-21 14:48   ` Will Deacon [this message]
2020-02-21 23:44     ` Masami Hiramatsu
2020-02-21 15:41 ` David Laight
2020-02-21 16:25   ` Quentin Perret
2020-02-25 10:05 ` 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
2020-03-02 19:28 ` Mathieu Desnoyers
2020-03-02 19:36   ` Greg Kroah-Hartman
2020-03-02 19:39     ` Greg Kroah-Hartman
2020-03-02 20:17       ` Mathieu Desnoyers
2020-03-03  6:57         ` Greg Kroah-Hartman
2020-03-03 16:58           ` Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200221144853.GA18153@willie-the-truck \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.