Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Mon, 16 Apr 2018 08:13:16 -0700 | Subject | Re: Wrong module .text address in 4.16.0 |
| |
On Mon, Apr 16, 2018 at 6:43 AM, Jessica Yu <jeyu@kernel.org> 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) | |