Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org --- kernel/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..40b42000bd80 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1472,7 +1472,8 @@ 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); + return sprintf(buf, "0x%px\n", kptr_restrict < 2 ? + (void *)sattr->address : NULL); }
static void free_sect_attrs(struct module_sect_attrs *sect_attrs)
On Wed, Apr 18, 2018 at 09:14:36AM +0200, Thomas Richter wrote:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
What's changed in each version please?
thanks, Tobin.
On 04/18/2018 09:17 AM, Tobin C. Harding wrote:
On Wed, Apr 18, 2018 at 09:14:36AM +0200, Thomas Richter wrote:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
What's changed in each version please?
thanks, Tobin.
V2: Changed sprintf format string from %#lx to 0x%px (suggested by Kees Cook). V3: Changed sprintf agrument from 0 to NULL to avoid sparse warning.
+++ Thomas Richter [18/04/18 09:14 +0200]:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
Thanks for turning this into a real patch. Looks good to me, if Linus wants to take it directly:
Acked-by: Jessica Yu jeyu@kernel.org
Otherwise I can take this up my tree.
As Linus mentioned, we should technically be checking the opening task's credentials/capabilities, but converting everything to pass the actual struct seq_file/file to the sysfs show handlers would be fairly painful :/ At least those /sys/module/*/sections/ files are all set to 0400. I think this fix is sufficient for now.
kernel/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..40b42000bd80 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1472,7 +1472,8 @@ 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);
- return sprintf(buf, "0x%px\n", kptr_restrict < 2 ?
(void *)sattr->address : NULL);
}
static void free_sect_attrs(struct module_sect_attrs *sect_attrs)
2.14.3
On Wed, Apr 18, 2018 at 6:21 AM, Jessica Yu jeyu@kernel.org wrote:
Otherwise I can take this up my tree.
Take it through your tree.
It's not some recent thing that stops people from testing other stuff (which is when I tend to try to take it directly just to short-circuit any delays), so it might as well go through the "proper channels".
Thanks,
Linus
On Wed, Apr 18, 2018 at 12:14 AM, Thomas Richter tmricht@linux.ibm.com wrote:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
kernel/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..40b42000bd80 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1472,7 +1472,8 @@ 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);
return sprintf(buf, "0x%px\n", kptr_restrict < 2 ?
(void *)sattr->address : NULL);
Errr... this looks reversed to me.
I would expect: "kptr_restrict < 2 ? NULL : (void *)sattr->address"
-Kees
On 05/02/2018 04:20 AM, Kees Cook wrote:
On Wed, Apr 18, 2018 at 12:14 AM, Thomas Richter tmricht@linux.ibm.com wrote:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
kernel/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..40b42000bd80 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1472,7 +1472,8 @@ 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);
return sprintf(buf, "0x%px\n", kptr_restrict < 2 ?
(void *)sattr->address : NULL);
Errr... this looks reversed to me.
I would expect: "kptr_restrict < 2 ? NULL : (void *)sattr->address"
-Kees
I am confused: In my patch, if kptr_restrict == 2 it prints NULL, which kptr_restrict being 0 or 1 it prints the address.
In your comment if kptr_restrict == 2 it prints the address, which kptr_restrict being 0 or 1 it prints NULL.
Looking into Documentation/sysctl/kernel.txt: When kptr_restrict is set to (2), kernel pointers printed using %pK will be replaced with 0's regardless of privileges.
With my patch, setting kptr_restrict to 0 or 1 prints the real kernel address (format %px, unmodified address according to Documentation/printk-formats.txt).
I have tested this on s390 (which is the only arch using file /sys/module/<XXX>/sections/.text) in the perf tool.
root@s8360047 ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0 [root@s8360047 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 102400 1 - Live 0x000003ff8034d000 [root@s8360047 ~]# cat /sys/module/qeth_l2/sections/.text 0x000003ff8034da68 [root@s8360047 ~]# sysctl -w kernel.kptr_restrict=2 kernel.kptr_restrict = 2 [root@s8360047 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 102400 1 - Live 0x0000000000000000 [root@s8360047 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000000000000 [root@s8360047 ~]# uname -a Linux s8360047 4.17.0-rc3m-perf+ #6 SMP PREEMPT Wed May 2 10:02:38 CEST 2018 s390x s390x s390x GNU/Linux [root@s8360047 ~]#
Hope this helps.
On Wed, May 2, 2018 at 1:13 AM, Thomas-Mich Richter tmricht@linux.ibm.com wrote:
On 05/02/2018 04:20 AM, Kees Cook wrote:
On Wed, Apr 18, 2018 at 12:14 AM, Thomas Richter tmricht@linux.ibm.com wrote:
Reading file /proc/modules shows the correct address: [root@s35lp76 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 94208 1 - Live 0x000003ff80401000
and reading file /sys/module/qeth_l2/sections/.text [root@s35lp76 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000018ea8363 displays a random address.
This breaks the perf tool which uses this address on s390 to calculate start of .text section in memory.
Fix this by printing the correct (unhashed) address.
Thanks to Jessica Yu for helping on this.
Fixes: ef0010a30935 ("vsprintf: don't use 'restricted_pointer()' when not restricting") Cc: stable@vger.kernel.org # v4.15+ Suggested-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Thomas Richter tmricht@linux.ibm.com Cc: Jessica Yu jeyu@kernel.org
kernel/module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/module.c b/kernel/module.c index a6e43a5806a1..40b42000bd80 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1472,7 +1472,8 @@ 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);
return sprintf(buf, "0x%px\n", kptr_restrict < 2 ?
(void *)sattr->address : NULL);
Errr... this looks reversed to me.
I would expect: "kptr_restrict < 2 ? NULL : (void *)sattr->address"
-Kees
I am confused: In my patch, if kptr_restrict == 2 it prints NULL, which kptr_restrict being 0 or 1 it prints the address.
In your comment if kptr_restrict == 2 it prints the address, which kptr_restrict being 0 or 1 it prints NULL.
Looking into Documentation/sysctl/kernel.txt: When kptr_restrict is set to (2), kernel pointers printed using %pK will be replaced with 0's regardless of privileges.
With my patch, setting kptr_restrict to 0 or 1 prints the real kernel address (format %px, unmodified address according to Documentation/printk-formats.txt).
I have tested this on s390 (which is the only arch using file /sys/module/<XXX>/sections/.text) in the perf tool.
root@s8360047 ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0 [root@s8360047 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 102400 1 - Live 0x000003ff8034d000 [root@s8360047 ~]# cat /sys/module/qeth_l2/sections/.text 0x000003ff8034da68 [root@s8360047 ~]# sysctl -w kernel.kptr_restrict=2 kernel.kptr_restrict = 2 [root@s8360047 ~]# cat /proc/modules | egrep '^qeth_l2' qeth_l2 102400 1 - Live 0x0000000000000000 [root@s8360047 ~]# cat /sys/module/qeth_l2/sections/.text 0x0000000000000000 [root@s8360047 ~]# uname -a Linux s8360047 4.17.0-rc3m-perf+ #6 SMP PREEMPT Wed May 2 10:02:38 CEST 2018 s390x s390x s390x GNU/Linux [root@s8360047 ~]#
Hope this helps.
Thanks! Yes, I was looking at too many of the %px commits in a row and confused myself. Sorry for the noise!
-Kees
linux-stable-mirror@lists.linaro.org