It is useful at least for debugging to have the kernel virtual
memory layout printed at boot time so to have the full information
about the booted kernel. Make the printing optional and available
only when DEBUG_KERNEL config is enabled so not to leak a sensitive
kernel information.
Signed-off-by: Serge Semin <[email protected]>
---
arch/mips/mm/init.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index bbb196ad5f26..c338bbd03b2a 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -31,6 +31,7 @@
#include <linux/gfp.h>
#include <linux/kcore.h>
#include <linux/initrd.h>
+#include <linux/sizes.h>
#include <asm/bootinfo.h>
#include <asm/cachectl.h>
@@ -56,6 +57,53 @@ unsigned long empty_zero_page, zero_page_mask;
EXPORT_SYMBOL_GPL(empty_zero_page);
EXPORT_SYMBOL(zero_page_mask);
+/*
+ * Print out the kernel virtual memory layout
+ */
+#define MLK(b, t) (void *)b, (void *)t, ((t) - (b)) >> 10
+#define MLM(b, t) (void *)b, (void *)t, ((t) - (b)) >> 20
+#define MLK_ROUNDUP(b, t) (void *)b, (void *)t, DIV_ROUND_UP(((t) - (b)), SZ_1K)
+static void __init mem_print_kmap_info(void)
+{
+#ifdef CONFIG_DEBUG_KERNEL
+ pr_notice("Kernel virtual memory layout:\n"
+ " lowmem : 0x%px - 0x%px (%4ld MB)\n"
+ " .text : 0x%px - 0x%px (%4td kB)\n"
+ " .data : 0x%px - 0x%px (%4td kB)\n"
+ " .init : 0x%px - 0x%px (%4td kB)\n"
+ " .bss : 0x%px - 0x%px (%4td kB)\n"
+ " vmalloc : 0x%px - 0x%px (%4ld MB)\n"
+#ifdef CONFIG_HIGHMEM
+ " pkmap : 0x%px - 0x%px (%4ld MB)\n"
+#endif
+ " fixmap : 0x%px - 0x%px (%4ld kB)\n",
+ MLM(PAGE_OFFSET, (unsigned long)high_memory),
+ MLK_ROUNDUP(_text, _etext),
+ MLK_ROUNDUP(_sdata, _edata),
+ MLK_ROUNDUP(__init_begin, __init_end),
+ MLK_ROUNDUP(__bss_start, __bss_stop),
+ MLM(VMALLOC_START, VMALLOC_END),
+#ifdef CONFIG_HIGHMEM
+ MLM(PKMAP_BASE, (PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE)),
+#endif
+ MLK(FIXADDR_START, FIXADDR_TOP));
+
+ /* Check some fundamental inconsistencies. May add something else? */
+#ifdef CONFIG_HIGHMEM
+ BUILD_BUG_ON(VMALLOC_END < PAGE_OFFSET);
+ BUG_ON(VMALLOC_END < (unsigned long)high_memory);
+ BUILD_BUG_ON((PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE) < PAGE_OFFSET);
+ BUG_ON((PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE) <
+ (unsigned long)high_memory);
+#endif
+ BUILD_BUG_ON(FIXADDR_TOP < PAGE_OFFSET);
+ BUG_ON(FIXADDR_TOP < (unsigned long)high_memory);
+#endif /* CONFIG_DEBUG_KERNEL */
+}
+#undef MLK
+#undef MLM
+#undef MLK_ROUNDUP
+
/*
* Not static inline because used by IP27 special magic initialization code
*/
@@ -479,6 +527,7 @@ void __init mem_init(void)
setup_zero_pages(); /* Setup zeroed pages. */
mem_init_free_highmem();
mem_init_print_info(NULL);
+ mem_print_kmap_info();
#ifdef CONFIG_64BIT
if ((unsigned long) &_text > (unsigned long) CKSEG0)
--
2.21.0
Hi Serge,
On Fri, May 03, 2019 at 08:50:39PM +0300, Serge Semin wrote:
> It is useful at least for debugging to have the kernel virtual
> memory layout printed at boot time so to have the full information
> about the booted kernel. Make the printing optional and available
> only when DEBUG_KERNEL config is enabled so not to leak a sensitive
> kernel information.
>
> Signed-off-by: Serge Semin <[email protected]>
> ---
> arch/mips/mm/init.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
FYI the rest of the series is in mips-next, but I left this one out
because it gives me compile errors for 64r6el_defconfig:
In file included from ./include/linux/printk.h:7,
from ./include/linux/kernel.h:15,
from ./include/asm-generic/bug.h:18,
from ./arch/mips/include/asm/bug.h:42,
from ./include/linux/bug.h:5,
from arch/mips/mm/init.c:11:
arch/mips/mm/init.c: In function ‘mem_print_kmap_info’:
./include/linux/kern_levels.h:5:18: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘long long unsigned int’ [-Werror=format=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
./include/linux/kern_levels.h:13:21: note: in expansion of macro ‘KERN_SOH’
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
./include/linux/printk.h:307:9: note: in expansion of macro ‘KERN_NOTICE’
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/mips/mm/init.c:69:2: note: in expansion of macro ‘pr_notice’
pr_notice("Kernel virtual memory layout:\n"
^~~~~~~~~
arch/mips/mm/init.c:70:39: note: format string is defined here
" lowmem : 0x%px - 0x%px (%4ld MB)\n"
~~~^
%4lld
In file included from ./arch/mips/include/asm/bug.h:5,
from ./include/linux/bug.h:5,
from arch/mips/mm/init.c:11:
In function ‘mem_print_kmap_info’,
inlined from ‘mem_init’ at arch/mips/mm/init.c:530:2:
./include/linux/compiler.h:344:38: error: call to ‘__compiletime_assert_99’ declared with attribute error: BUILD_BUG_ON failed: FIXADDR_TOP < PAGE_OFFSET
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
./include/linux/compiler.h:325:4: note: in definition of macro ‘__compiletime_assert’
prefix ## suffix(); \
^~~~~~
./include/linux/compiler.h:344:2: note: in expansion of macro ‘_compiletime_assert’
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
arch/mips/mm/init.c:99:2: note: in expansion of macro ‘BUILD_BUG_ON’
BUILD_BUG_ON(FIXADDR_TOP < PAGE_OFFSET);
^~~~~~~~~~~~
cc1: all warnings being treated as errors
make[3]: *** [scripts/Makefile.build:278: arch/mips/mm/init.o] Error 1
make[2]: *** [scripts/Makefile.build:489: arch/mips/mm] Error 2
Thanks,
Paul
Hello Paul
On Mon, May 06, 2019 at 07:14:21PM +0000, Paul Burton wrote:
> Hi Serge,
>
> On Fri, May 03, 2019 at 08:50:39PM +0300, Serge Semin wrote:
> > It is useful at least for debugging to have the kernel virtual
> > memory layout printed at boot time so to have the full information
> > about the booted kernel. Make the printing optional and available
> > only when DEBUG_KERNEL config is enabled so not to leak a sensitive
> > kernel information.
> >
> > Signed-off-by: Serge Semin <[email protected]>
> > ---
> > arch/mips/mm/init.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
>
> FYI the rest of the series is in mips-next, but I left this one out
> because it gives me compile errors for 64r6el_defconfig:
>
> In file included from ./include/linux/printk.h:7,
> from ./include/linux/kernel.h:15,
> from ./include/asm-generic/bug.h:18,
> from ./arch/mips/include/asm/bug.h:42,
> from ./include/linux/bug.h:5,
> from arch/mips/mm/init.c:11:
> arch/mips/mm/init.c: In function ‘mem_print_kmap_info’:
> ./include/linux/kern_levels.h:5:18: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘long long unsigned int’ [-Werror=format=]
> #define KERN_SOH "\001" /* ASCII Start Of Header */
> ^~~~~~
> ./include/linux/kern_levels.h:13:21: note: in expansion of macro ‘KERN_SOH’
> #define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
> ^~~~~~~~
> ./include/linux/printk.h:307:9: note: in expansion of macro ‘KERN_NOTICE’
> printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
> ^~~~~~~~~~~
> arch/mips/mm/init.c:69:2: note: in expansion of macro ‘pr_notice’
> pr_notice("Kernel virtual memory layout:\n"
> ^~~~~~~~~
> arch/mips/mm/init.c:70:39: note: format string is defined here
> " lowmem : 0x%px - 0x%px (%4ld MB)\n"
> ~~~^
> %4lld
> In file included from ./arch/mips/include/asm/bug.h:5,
> from ./include/linux/bug.h:5,
> from arch/mips/mm/init.c:11:
> In function ‘mem_print_kmap_info’,
> inlined from ‘mem_init’ at arch/mips/mm/init.c:530:2:
> ./include/linux/compiler.h:344:38: error: call to ‘__compiletime_assert_99’ declared with attribute error: BUILD_BUG_ON failed: FIXADDR_TOP < PAGE_OFFSET
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> ^
> ./include/linux/compiler.h:325:4: note: in definition of macro ‘__compiletime_assert’
> prefix ## suffix(); \
> ^~~~~~
> ./include/linux/compiler.h:344:2: note: in expansion of macro ‘_compiletime_assert’
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> ^~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
> #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> ^~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
> BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> ^~~~~~~~~~~~~~~~
> arch/mips/mm/init.c:99:2: note: in expansion of macro ‘BUILD_BUG_ON’
> BUILD_BUG_ON(FIXADDR_TOP < PAGE_OFFSET);
> ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[3]: *** [scripts/Makefile.build:278: arch/mips/mm/init.o] Error 1
> make[2]: *** [scripts/Makefile.build:489: arch/mips/mm] Error 2
>
> Thanks,
> Paul
Thanks for the report regarding this issue. I actually thought I tested the patch
being buildable for 64bit systems. It turns out I didn't.(
Should I resend the fixed patch as a separate v3 one In-Reply-to this v2 patch
or resubmit the patchset with cover-letter and only the fixed patch being there?
Cheers,
-Sergey
Hi Serge,
On Wed, May 08, 2019 at 01:36:07AM +0300, Serge Semin wrote:
> Thanks for the report regarding this issue. I actually thought I
> tested the patch being buildable for 64bit systems. It turns out I
> didn't.(
Easily done :)
> Should I resend the fixed patch as a separate v3 one In-Reply-to this
> v2 patch or resubmit the patchset with cover-letter and only the fixed
> patch being there?
Replying with just v3 of this patch will be fine, no need to resend the
cover letter.
I currently plan to submit a pull request for mips-next as-is, without
this patch, in the next day or two. There are a few last minute
submissions this time round that I'll then queue up & send a second pull
request next week, which this can be part of.
Thanks,
Paul
It is useful at least for debugging to have the kernel virtual
memory layout printed at boot time so to have the full information
about the booted kernel. Make the printing optional and available
only when DEBUG_KERNEL config is enabled so not to leak a sensitive
kernel information.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v3
- Add MLM_ROUNDUP() and use it to calculate the low memory addresses range.
- Print KB instead of kB for kilobyte quantities.
- Check constants inconsistancies for systems with HIGHMEM only.
---
arch/mips/mm/init.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index bbb196ad5f26..40558b979f96 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -31,6 +31,7 @@
#include <linux/gfp.h>
#include <linux/kcore.h>
#include <linux/initrd.h>
+#include <linux/sizes.h>
#include <asm/bootinfo.h>
#include <asm/cachectl.h>
@@ -56,6 +57,55 @@ unsigned long empty_zero_page, zero_page_mask;
EXPORT_SYMBOL_GPL(empty_zero_page);
EXPORT_SYMBOL(zero_page_mask);
+/*
+ * Print out the kernel virtual memory layout
+ */
+#define MLK(b, t) (void *)b, (void *)t, ((t) - (b)) >> 10
+#define MLM(b, t) (void *)b, (void *)t, ((t) - (b)) >> 20
+#define MLK_ROUNDUP(b, t) (void *)b, (void *)t, DIV_ROUND_UP(((t) - (b)), SZ_1K)
+#define MLM_ROUNDUP(b, t) (void *)b, (void *)t, DIV_ROUND_UP(((t) - (b)), SZ_1M)
+static void __init mem_print_kmap_info(void)
+{
+#ifdef CONFIG_DEBUG_KERNEL
+ pr_notice("Kernel virtual memory layout:\n"
+ " lowmem : 0x%px - 0x%px (%6td MB)\n"
+ " .text : 0x%px - 0x%px (%6td KB)\n"
+ " .data : 0x%px - 0x%px (%6td KB)\n"
+ " .init : 0x%px - 0x%px (%6td KB)\n"
+ " .bss : 0x%px - 0x%px (%6td KB)\n"
+ " vmalloc : 0x%px - 0x%px (%6ld MB)\n"
+#ifdef CONFIG_HIGHMEM
+ " pkmap : 0x%px - 0x%px (%6ld MB)\n"
+#endif
+ " fixmap : 0x%px - 0x%px (%6ld KB)\n",
+ MLM_ROUNDUP((void *)PAGE_OFFSET, high_memory),
+ MLK_ROUNDUP(_text, _etext),
+ MLK_ROUNDUP(_sdata, _edata),
+ MLK_ROUNDUP(__init_begin, __init_end),
+ MLK_ROUNDUP(__bss_start, __bss_stop),
+ MLM(VMALLOC_START, VMALLOC_END),
+#ifdef CONFIG_HIGHMEM
+ MLM(PKMAP_BASE, (PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE)),
+#endif
+ MLK(FIXADDR_START, FIXADDR_TOP));
+
+ /* Check some fundamental inconsistencies. May add something else? */
+#ifdef CONFIG_HIGHMEM
+ BUILD_BUG_ON(VMALLOC_END < PAGE_OFFSET);
+ BUG_ON(VMALLOC_END < (unsigned long)high_memory);
+ BUILD_BUG_ON((PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE) < PAGE_OFFSET);
+ BUG_ON((PKMAP_BASE) + (LAST_PKMAP)*(PAGE_SIZE) <
+ (unsigned long)high_memory);
+ BUILD_BUG_ON(FIXADDR_TOP < PAGE_OFFSET);
+ BUG_ON(FIXADDR_TOP < (unsigned long)high_memory);
+#endif
+#endif /* CONFIG_DEBUG_KERNEL */
+}
+#undef MLK
+#undef MLM
+#undef MLK_ROUNDUP
+#undef MLM_ROUNDUP
+
/*
* Not static inline because used by IP27 special magic initialization code
*/
@@ -479,6 +529,7 @@ void __init mem_init(void)
setup_zero_pages(); /* Setup zeroed pages. */
mem_init_free_highmem();
mem_init_print_info(NULL);
+ mem_print_kmap_info();
#ifdef CONFIG_64BIT
if ((unsigned long) &_text > (unsigned long) CKSEG0)
--
2.21.0
On Tue, May 07, 2019 at 10:41:10PM +0000, Paul Burton wrote:
> Hi Serge,
>
> On Wed, May 08, 2019 at 01:36:07AM +0300, Serge Semin wrote:
> > Thanks for the report regarding this issue. I actually thought I
> > tested the patch being buildable for 64bit systems. It turns out I
> > didn't.(
>
> Easily done :)
>
> > Should I resend the fixed patch as a separate v3 one In-Reply-to this
> > v2 patch or resubmit the patchset with cover-letter and only the fixed
> > patch being there?
>
> Replying with just v3 of this patch will be fine, no need to resend the
> cover letter.
>
Ok. I've just submitted the v3 version with fixed buildability problem.
> I currently plan to submit a pull request for mips-next as-is, without
> this patch, in the next day or two. There are a few last minute
> submissions this time round that I'll then queue up & send a second pull
> request next week, which this can be part of.
>
> Thanks,
> Paul
Regarding this patch being part of the mips mm init code. I've just found out
that 32-bit arm subsystem maintainers removed the same functionality from the
kernel 5.1. This also was removed from arm64 in kernel 4.15:
commit 1c31d4e96b8c ("ARM: 8820/1: mm: Stop printing the virtual memory layout")
commit 071929dbdd86 ("arm64: Stop printing the virtual memory layout")
Maintainer of m68k and unicore32 discarded the printing as well:
commit 1476ea250cf0 ("unicore32: stop printing the virtual memory layout")
commit 31833332f798 ("m68k/mm: Stop printing the virtual memory layout")
The reasoning of these removal was that since commit ad67b74d2469 ("printk:
hash addresses printed with %p") the kernel virtual addresses weren't
printed to the system log anyway. So instead of replacing the format string with
"%px" they decided not to leak a virtual memory layout information and completely
removed the printing. I don't really know why they didn't closed the printing for
debug kernel only as we did, since the info might be useful in this case.
Since I see a tendency of this functionality removal, we might need to
reconsider this patch integration into the MIPS arch code. What do you think?
Although some architectures still perform the virtual memory layout printing
at boot-time: x86_32, parisc, xtensa, sh, nds32 (might be others).
Cheers,
-Sergey
Hello Paul
On Wed, May 08, 2019 at 02:38:49AM +0300, Serge Semin wrote:
> On Tue, May 07, 2019 at 10:41:10PM +0000, Paul Burton wrote:
> > Hi Serge,
> >
> > On Wed, May 08, 2019 at 01:36:07AM +0300, Serge Semin wrote:
> > > Thanks for the report regarding this issue. I actually thought I
> > > tested the patch being buildable for 64bit systems. It turns out I
> > > didn't.(
> >
> > Easily done :)
> >
> > > Should I resend the fixed patch as a separate v3 one In-Reply-to this
> > > v2 patch or resubmit the patchset with cover-letter and only the fixed
> > > patch being there?
> >
> > Replying with just v3 of this patch will be fine, no need to resend the
> > cover letter.
> >
>
> Ok. I've just submitted the v3 version with fixed buildability problem.
>
> > I currently plan to submit a pull request for mips-next as-is, without
> > this patch, in the next day or two. There are a few last minute
> > submissions this time round that I'll then queue up & send a second pull
> > request next week, which this can be part of.
> >
> > Thanks,
> > Paul
>
> Regarding this patch being part of the mips mm init code. I've just found out
> that 32-bit arm subsystem maintainers removed the same functionality from the
> kernel 5.1. This also was removed from arm64 in kernel 4.15:
> commit 1c31d4e96b8c ("ARM: 8820/1: mm: Stop printing the virtual memory layout")
> commit 071929dbdd86 ("arm64: Stop printing the virtual memory layout")
>
> Maintainer of m68k and unicore32 discarded the printing as well:
> commit 1476ea250cf0 ("unicore32: stop printing the virtual memory layout")
> commit 31833332f798 ("m68k/mm: Stop printing the virtual memory layout")
>
> The reasoning of these removal was that since commit ad67b74d2469 ("printk:
> hash addresses printed with %p") the kernel virtual addresses weren't
> printed to the system log anyway. So instead of replacing the format string with
> "%px" they decided not to leak a virtual memory layout information and completely
> removed the printing. I don't really know why they didn't closed the printing for
> debug kernel only as we did, since the info might be useful in this case.
>
> Since I see a tendency of this functionality removal, we might need to
> reconsider this patch integration into the MIPS arch code. What do you think?
>
> Although some architectures still perform the virtual memory layout printing
> at boot-time: x86_32, parisc, xtensa, sh, nds32 (might be others).
>
> Cheers,
> -Sergey
So any update on this patch status?
Regards,
-Sergey