Message-ID: <20200221144853.GA18153@willie-the-truck>
Date:   Fri, 21 Feb 2020 14:48:54 +0000
From:   Will Deacon <will@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     linux-kernel@...r.kernel.org, kernel-team@...roid.com,
        akpm@...ux-foundation.org,
        "K . Prasad" <prasad@...ux.vnet.ibm.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Christoph Hellwig <hch@....de>,
        Quentin Perret <qperret@...gle.com>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and
 kallsyms_on_each_symbol()

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 <will@...nel.org> 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.

Will

Powered by blists - more mailing lists