Messages in this thread | | | Date | Sat, 22 Feb 2020 08:44:38 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol() |
| |
On Fri, 21 Feb 2020 14:48:54 +0000 Will Deacon <will@kernel.org> wrote:
> 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@kernel.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.
So this is to officially states our policy that if out-of-tree driver developers need some symbol exposed, they should work with upstream to find better solution. Not for fixing some kind of security hole.
> > 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.
Fair enough.
> > > 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?
No, I don't. :)
> > > 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.
Agreed. Just for discouraging abuse of unexposed symbols, I think this is enough.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
for thise series.
Thank you,
-- Masami Hiramatsu <mhiramat@kernel.org>
| |