Re: Wrong module .text address in 4.16.0

[Linus Torvalds]

On Mon, Apr 16, 2018 at 6:43 AM, Jessica Yu <jeyu@xxxxxxxxxx> wrote:
>
> So for users of /sys/module/*/sections, we will need to work around
> this and possibly use %px for the real address. But perhaps we should
> base the usage of %px on kptr_restrict?

Maybe. I was hoping we would be able to get rid of it eventually.

The real problem is that those darn module_attribute things don't have
proper IO routines. They *only* have the show routine, and that
doesn't even get the 'struct file' pointer passed to it, just the
buffer to fill in (not even a _size_ of a buffer - we're talking the
bad bad old days of nasty /proc interfaces).

Why is that a problem? Without a 'struct file' we can't even do
permission checking right. %pK worked by doing disgusting wrong
things.

Now, in this case, at least the files are root-owned, and legible only
to root, so I guess we can say that permissions have been properly
checked at open time (not really true: the CAP_SYSLOG bit wasn't!, but
I doubt anybody really cares), and so we could just check
kptr_restrict.

Oh well.

Something like the attached, perhaps? Completely untested, and I don't
even want credit for this if it is used.

              Linus
 kernel/module.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index a6e43a5806a1..f8cf0bb35ab6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1472,7 +1472,11 @@ static ssize_t module_sect_show(struct module_attribute *mattr,
 {
 	struct module_sect_attr *sattr =
 		container_of(mattr, struct module_sect_attr, mattr);
-	return sprintf(buf, "0x%pK\n", (void *)sattr->address);
+	unsigned long addr = 0;
+
+	/* Permissions were checked at open */
+	addr = kptr_restrict < 2 ?sattr->address : 0;
+	return sprintf(buf, "%#lx\n", addr);
 }
 
 static void free_sect_attrs(struct module_sect_attrs *sect_attrs)