Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()

Given the name of a kernel symbol, the 'data_breakpoint' test claims to "report any write operations on the kernel symbol". However, it creates the breakpoint using both HW_BREAKPOINT_W and HW_BREAKPOINT_R, which menas it also fires for read access.

Drop HW_BREAKPOINT_R from the breakpoint attributes.

samples/hw_breakpoint/data_breakpoint.c line 26

Quoted 2 lines · Will Deacon

samples/hw_breakpoint/data_breakpoint.c +1 -1

-static char ksym_name[KSYM_NAME_LEN] = "pid_max";
+static char ksym_name[KSYM_NAME_LEN] = "jiffies";

Is jiffies actually an exported symbol on all configfs? I thought there was some weird aliasing going on with jiffies64.

Except for the symbol choice this looks fine, though:

Reviewed-by Christoph Hellwig <[email protected]>

kallsyms_lookup_name() and kallsyms_on_each_symbol() are exported to modules despite having no in-tree users and being wide open to abuse by out-of-tree modules that can use them as a method to invoke arbitrary non-exported kernel functions.

Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol().

Cc Alexei Starovoitov <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Quentin Perret <[email protected]>

Signed-off-by Will Deacon <[email protected]>

kallsyms_lookup_name() and kallsyms_on_each_symbol() are exported to modules despite having no in-tree users and being wide open to abuse by out-of-tree modules that can use them as a method to invoke arbitrary non-exported kernel functions.

Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol().

kallsyms_lookup_name() and kallsyms_on_each_symbol() are exported to modules despite having no in-tree users and being wide open to abuse by out-of-tree modules that can use them as a method to invoke arbitrary non-exported kernel functions.

Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol().

Cc Alexei Starovoitov <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Quentin Perret <[email protected]>

Signed-off-by Will Deacon <[email protected]>

---
 kernel/kallsyms.c | 2 --
 1 file changed, 2 deletions(-)

kernel/kallsyms.c +0 -1

index a9b3f660dee7..16c8c605f4b0 100644
@@ -175,7 +175,6 @@ unsigned long kallsyms_lookup_name(const char *name)
 	}
 	return module_kallsyms_lookup_name(name);
 }
-EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
 
 int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 				      unsigned long),

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.

Quoted 5 lines · On Fri, 21 Feb 2020 11:44:01 +0000 Will Deacon <[email protected]> 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.

Quoted 3 lines

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?

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.

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

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?

Hi Masami,

Quoted 7 lines · Masami Hiramatsu

Quoted 5 lines · On Fri, 21 Feb 2020 11:44:01 +0000 Will Deacon <[email protected]> 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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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?

Quoted 4 lines

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.

Sent: 21 February 2020 11:44
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.

From: Will Deacon

Quoted 7 lines

Sent: 21 February 2020 11:44
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.

Now where did I put that kernel code that opens /proc/kallsyms and then reads it to find the addresses of symbols ...

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 <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Quentin Perret <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Masami Hiramatsu <[email protected]>

--->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(-)

Signature · 1 line

2.25.0.265.gbab2e86ba0-goog

CC live-patching ML, because this could affect many of its users...

Quoted 6 lines · Will Deacon

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.

Quoted 10 lines · Miroslav Benes

CC live-patching ML, because this could affect many of its users...

Quoted 6 lines · Will Deacon

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/[email protected]

I am not sure if this use case is enough to keep the symbols exported. Anyway, there are currently some out-of-tree users.

Quoted 20 lines · Petr Mladek

Quoted 10 lines · Miroslav Benes

CC live-patching ML, because this could affect many of its users...

Quoted 6 lines · Will Deacon

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.

Quoted 4 lines

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.

Quoted 12 lines

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/[email protected]

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.

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

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.

Quoted 10 lines · Will Deacon

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.

Hi,

I maintain a GPL kernel tracer (LTTng) since 2005 which happens to be out-of-tree, even though we have made unsuccessful attempts to upstream it in the past. It uses kallsyms_lookup_name() to fetch a few symbols. I would be very glad to have them GPL-exported upstream rather than relying on this work-around. Here is the list of symbols we would need to GPL-export:

stack_trace_save
stack_trace_save_user
vmalloc_sync_all (CONFIG_X86)
get_pfnblock_flags_mask
disk_name
block_class
disk_type

Quoted 21 lines · Mathieu Desnoyers

Quoted 10 lines · Will Deacon

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.

Hi,

I maintain a GPL kernel tracer (LTTng) since 2005 which happens to be out-of-tree, even though we have made unsuccessful attempts to upstream it in the past. It uses kallsyms_lookup_name() to fetch a few symbols. I would be very glad to have them GPL-exported upstream rather than relying on this work-around. Here is the list of symbols we would need to GPL-export:

Quoted 7 lines

stack_trace_save
stack_trace_save_user
vmalloc_sync_all (CONFIG_X86)
get_pfnblock_flags_mask
disk_name
block_class
disk_type

I hate to ask, but why does anyone need block_class? or disk_name or disk_type? I need to put them behind a driver core namespace or something soon...

Quoted 30 lines · Greg Kroah-Hartman

Quoted 28 lines · Mathieu Desnoyers

Quoted 10 lines · Will Deacon

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.

Hi,

I maintain a GPL kernel tracer (LTTng) since 2005 which happens to be out-of-tree, even though we have made unsuccessful attempts to upstream it in the past. It uses kallsyms_lookup_name() to fetch a few symbols. I would be very glad to have them GPL-exported upstream rather than relying on this work-around. Here is the list of symbols we would need to GPL-export:

stack_trace_save
stack_trace_save_user
vmalloc_sync_all (CONFIG_X86)
get_pfnblock_flags_mask
disk_name
block_class
disk_type

Quoted 3 lines

I hate to ask, but why does anyone need block_class? or disk_name or disk_type? I need to put them behind a driver core namespace or something soon...

Wait, disk_type is a static variable. And there's multiple ones of them, how does that work?

Quoted 31 lines · ----- On Mar 2, 2020, at 2:39 PM, Greg Kroah-Hartman…

Quoted 30 lines · Greg Kroah-Hartman

Quoted 28 lines · Mathieu Desnoyers

Quoted 10 lines · Will Deacon

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.

Hi,

I maintain a GPL kernel tracer (LTTng) since 2005 which happens to be out-of-tree, even though we have made unsuccessful attempts to upstream it in the past. It uses kallsyms_lookup_name() to fetch a few symbols. I would be very glad to have them GPL-exported upstream rather than relying on this work-around. Here is the list of symbols we would need to GPL-export:

stack_trace_save
stack_trace_save_user
vmalloc_sync_all (CONFIG_X86)
get_pfnblock_flags_mask
disk_name
block_class
disk_type

Quoted 4 lines

Quoted 3 lines

I hate to ask, but why does anyone need block_class? or disk_name or disk_type? I need to put them behind a driver core namespace or something soon...

In LTTng, we have a "statedump" which dumps all the relevant state of the kernel at trace start (or when the user asks for it) into the kernel trace. It is used to collect information which helps translating compact numeric data into human-readable information at post-processing.

For block devices, the statedump contains the mapping between the device number and the disk name [1]. It uses the "block_class", "disk_name", and "disk_type" symbols for this. Here is the post-processing output:

[14:48:41.388934812] (+?.?????????) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 1048576, diskname = "ram0" } [...]
[14:48:41.442548745] (+0.003574998) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 1048591, diskname = "ram15" } [14:48:41.446064977] (+0.003516232) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289728, diskname = "vda" } [14:48:41.449579781] (+0.003514804) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289729, diskname = "vda1" } [14:48:41.453113808] (+0.003534027) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289744, diskname = "vdb" } [14:48:41.456640876] (+0.003527068) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289745, diskname = "vdb1" }

This information is then used in our I/O analyses to show information comprehensible to a user.

Quoted 2 lines

Wait, disk_type is a static variable. And there's multiple ones of them, how does that work?

Yes, this is far from ideal. Here are the ones I observe in the kernel sources:

block/genhd.c
40:static const struct device_type disk_type; <---- the one we use

lib/raid6/test/test.c
41:static char disk_type(int d) <---- this is a stand-alone user-space test program, not part of the kernel image.

crypto/async_tx/raid6test.c (depends on CONFIG_ASYNC_RAID6_TEST)
44:static char disk_type(int d, int disks) <---- the compiler optimizes away this function, so this symbol is not present in the kernel image.

I think a better approach to solve this would be to implement and expose an iterator function in the core kernel which could invoke a callback. However, the main issue remains: if the only user is out-of-tree, I cannot justify adding an exported kernel helper for this.

Quoted 4 lines · Mathieu Desnoyers

----- On Mar 2, 2020, at 2:39 PM, Greg Kroah-Hartman [email protected]

Quoted 1 line · wrote:

On Mon, Mar 02, 2020 at 08:36:58PM +0100, Greg Kroah-Hartman wrote:

Quoted 30 lines

Quoted 5 lines

Quoted 4 lines

I hate to ask, but why does anyone need block_class? or disk_name or disk_type? I need to put them behind a driver core namespace or something soon...

In LTTng, we have a "statedump" which dumps all the relevant state of the kernel at trace start (or when the user asks for it) into the kernel trace. It is used to collect information which helps translating compact numeric data into human-readable information at post-processing.

For block devices, the statedump contains the mapping between the device number and the disk name [1]. It uses the "block_class", "disk_name", and "disk_type" symbols for this. Here is the post-processing output:

[14:48:41.388934812] (+?.?????????) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 1048576, diskname = "ram0" }
[...]
[14:48:41.442548745] (+0.003574998) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 1048591, diskname = "ram15" } [14:48:41.446064977] (+0.003516232) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289728, diskname = "vda" } [14:48:41.449579781] (+0.003514804) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289729, diskname = "vda1" } [14:48:41.453113808] (+0.003534027) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289744, diskname = "vdb" } [14:48:41.456640876] (+0.003527068) compudjdev lttng_statedump_block_device: { cpu_id = 0 }, { dev = 265289745, diskname = "vdb1" }

Quoted 2 lines

This information is then used in our I/O analyses to show information comprehensible to a user.

But all of that is availble to you today in userspace, why dig through random kernel symbols?

Look in /sys/dev/block/ or in /sys/block/ for all of that information. Is there something that you can only find by the internal symbols that is not present today in sysfs?