From: Will Deacon <email@example.com> To: Masami Hiramatsu <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com, "K . Prasad" <firstname.lastname@example.org>, Thomas Gleixner <email@example.com>, Greg Kroah-Hartman <firstname.lastname@example.org>, Frederic Weisbecker <email@example.com>, Christoph Hellwig <firstname.lastname@example.org>, Quentin Perret <email@example.com>, Alexei Starovoitov <firstname.lastname@example.org> 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: <email@example.com> 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 <firstname.lastname@example.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
next prev parent 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: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 0/3] Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * 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.