| Message ID | 20210330045132.722243-4-jniethe5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Further Strict RWX support | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/merge (87d76f542a24ecfa797e9bd3bb56c0f19aabff57) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/next (fbda7904302499dd7ffc073a3c84eb7c9275db0a) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linus/master (0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/fixes (cc7a0bb058b85ea03db87169c60c7cfdd5d34678) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linux-next (1df27313f50a57497c1faeb6a6ae4ca939c85a7d) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
Le 30/03/2021 à 06:51, Jordan Niethe a écrit : > If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and > VMALLOC_END respectively. This reduces the need for special cases. For > example, powerpc's module_alloc() was previously predicated on > MODULES_VADDR being defined but now is unconditionally defined. > > This will be useful reducing conditional code in other places that need > to allocate from the module region (i.e., kprobes). > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > v10: New to series > --- > arch/powerpc/include/asm/pgtable.h | 5 +++++ > arch/powerpc/kernel/module.c | 5 +---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > index 4eed82172e33..014c2921f26a 100644 > --- a/arch/powerpc/include/asm/pgtable.h > +++ b/arch/powerpc/include/asm/pgtable.h > @@ -167,6 +167,11 @@ struct seq_file; > void arch_report_meminfo(struct seq_file *m); > #endif /* CONFIG_PPC64 */ > > +#ifndef MODULES_VADDR > +#define MODULES_VADDR VMALLOC_START > +#define MODULES_END VMALLOC_END > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_PGTABLE_H */ > diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c > index a211b0253cdb..f1fb58389d58 100644 > --- a/arch/powerpc/kernel/module.c > +++ b/arch/powerpc/kernel/module.c > @@ -14,6 +14,7 @@ > #include <asm/firmware.h> > #include <linux/sort.h> > #include <asm/setup.h> > +#include <linux/mm.h> > > static LIST_HEAD(module_bug_list); > > @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, > return 0; > } > > -#ifdef MODULES_VADDR > void *module_alloc(unsigned long size) > { > - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); > - This check is important, if we remove it from here it should be done somewhere else, for instance in asm/task_size_32.h > return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, > PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > __builtin_return_address(0)); > } > -#endif >
Le 30/03/2021 à 06:51, Jordan Niethe a écrit : > If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and > VMALLOC_END respectively. This reduces the need for special cases. For > example, powerpc's module_alloc() was previously predicated on > MODULES_VADDR being defined but now is unconditionally defined. > > This will be useful reducing conditional code in other places that need > to allocate from the module region (i.e., kprobes). > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > v10: New to series > --- > arch/powerpc/include/asm/pgtable.h | 5 +++++ > arch/powerpc/kernel/module.c | 5 +---- You probably also have changes to do in kernel/ptdump.c In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here. > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > index 4eed82172e33..014c2921f26a 100644 > --- a/arch/powerpc/include/asm/pgtable.h > +++ b/arch/powerpc/include/asm/pgtable.h > @@ -167,6 +167,11 @@ struct seq_file; > void arch_report_meminfo(struct seq_file *m); > #endif /* CONFIG_PPC64 */ > > +#ifndef MODULES_VADDR > +#define MODULES_VADDR VMALLOC_START > +#define MODULES_END VMALLOC_END > +#endif > + > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_PGTABLE_H */ > diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c > index a211b0253cdb..f1fb58389d58 100644 > --- a/arch/powerpc/kernel/module.c > +++ b/arch/powerpc/kernel/module.c > @@ -14,6 +14,7 @@ > #include <asm/firmware.h> > #include <linux/sort.h> > #include <asm/setup.h> > +#include <linux/mm.h> > > static LIST_HEAD(module_bug_list); > > @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, > return 0; > } > > -#ifdef MODULES_VADDR > void *module_alloc(unsigned long size) > { > - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); > - The above check is needed somewhere, if you remove it from here you have to perform the check somewhere else. > return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, > PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > __builtin_return_address(0)); > } > -#endif >
On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 30/03/2021 à 06:51, Jordan Niethe a écrit : > > If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and > > VMALLOC_END respectively. This reduces the need for special cases. For > > example, powerpc's module_alloc() was previously predicated on > > MODULES_VADDR being defined but now is unconditionally defined. > > > > This will be useful reducing conditional code in other places that need > > to allocate from the module region (i.e., kprobes). > > > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > > --- > > v10: New to series > > --- > > arch/powerpc/include/asm/pgtable.h | 5 +++++ > > arch/powerpc/kernel/module.c | 5 +---- > > You probably also have changes to do in kernel/ptdump.c > > In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here. > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > > index 4eed82172e33..014c2921f26a 100644 > > --- a/arch/powerpc/include/asm/pgtable.h > > +++ b/arch/powerpc/include/asm/pgtable.h > > @@ -167,6 +167,11 @@ struct seq_file; > > void arch_report_meminfo(struct seq_file *m); > > #endif /* CONFIG_PPC64 */ > > > > +#ifndef MODULES_VADDR > > +#define MODULES_VADDR VMALLOC_START > > +#define MODULES_END VMALLOC_END > > +#endif > > + > > #endif /* __ASSEMBLY__ */ > > > > #endif /* _ASM_POWERPC_PGTABLE_H */ > > diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c > > index a211b0253cdb..f1fb58389d58 100644 > > --- a/arch/powerpc/kernel/module.c > > +++ b/arch/powerpc/kernel/module.c > > @@ -14,6 +14,7 @@ > > #include <asm/firmware.h> > > #include <linux/sort.h> > > #include <asm/setup.h> > > +#include <linux/mm.h> > > > > static LIST_HEAD(module_bug_list); > > > > @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, > > return 0; > > } > > > > -#ifdef MODULES_VADDR > > void *module_alloc(unsigned long size) > > { > > - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); > > - > > The above check is needed somewhere, if you remove it from here you have to perform the check > somewhere else. This also introduces this warning: fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to false [-Wtautological-compare] 626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) { I might leave this patch out of this series and use an #ifdef for now and make this change separately as a follow up. > > > return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, > > PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > > __builtin_return_address(0)); > > } > > -#endif > >
Le 21/04/2021 à 04:46, Jordan Niethe a écrit : > On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy > <christophe.leroy@csgroup.eu> wrote: >> >> >> >> Le 30/03/2021 à 06:51, Jordan Niethe a écrit : >>> If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and >>> VMALLOC_END respectively. This reduces the need for special cases. For >>> example, powerpc's module_alloc() was previously predicated on >>> MODULES_VADDR being defined but now is unconditionally defined. >>> >>> This will be useful reducing conditional code in other places that need >>> to allocate from the module region (i.e., kprobes). >>> >>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com> >>> --- >>> v10: New to series >>> --- >>> arch/powerpc/include/asm/pgtable.h | 5 +++++ >>> arch/powerpc/kernel/module.c | 5 +---- >> >> You probably also have changes to do in kernel/ptdump.c >> >> In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here. >> >>> 2 files changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h >>> index 4eed82172e33..014c2921f26a 100644 >>> --- a/arch/powerpc/include/asm/pgtable.h >>> +++ b/arch/powerpc/include/asm/pgtable.h >>> @@ -167,6 +167,11 @@ struct seq_file; >>> void arch_report_meminfo(struct seq_file *m); >>> #endif /* CONFIG_PPC64 */ >>> >>> +#ifndef MODULES_VADDR >>> +#define MODULES_VADDR VMALLOC_START >>> +#define MODULES_END VMALLOC_END >>> +#endif >>> + >>> #endif /* __ASSEMBLY__ */ >>> >>> #endif /* _ASM_POWERPC_PGTABLE_H */ >>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c >>> index a211b0253cdb..f1fb58389d58 100644 >>> --- a/arch/powerpc/kernel/module.c >>> +++ b/arch/powerpc/kernel/module.c >>> @@ -14,6 +14,7 @@ >>> #include <asm/firmware.h> >>> #include <linux/sort.h> >>> #include <asm/setup.h> >>> +#include <linux/mm.h> >>> >>> static LIST_HEAD(module_bug_list); >>> >>> @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, >>> return 0; >>> } >>> >>> -#ifdef MODULES_VADDR >>> void *module_alloc(unsigned long size) >>> { >>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); >>> - >> >> The above check is needed somewhere, if you remove it from here you have to perform the check >> somewhere else. > > This also introduces this warning: > fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to > false [-Wtautological-compare] > 626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) { > I might leave this patch out of this series and use an #ifdef for now > and make this change separately as a follow up. x86/32 at least does the same (see https://elixir.bootlin.com/linux/v5.12-rc8/source/arch/x86/include/asm/pgtable_32_areas.h#L47) They probably also get the warning, so I think would shouldn't bother. One day someone will fix fs/proc/kcore.c , that's not a powerpc problem. > >> >>> return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, >>> PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, >>> __builtin_return_address(0)); >>> } >>> -#endif >>>
On Wed, Apr 21, 2021 at 3:14 PM Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 21/04/2021 à 04:46, Jordan Niethe a écrit : > > On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy > > <christophe.leroy@csgroup.eu> wrote: > >> > >> > >> > >> Le 30/03/2021 à 06:51, Jordan Niethe a écrit : > >>> If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and > >>> VMALLOC_END respectively. This reduces the need for special cases. For > >>> example, powerpc's module_alloc() was previously predicated on > >>> MODULES_VADDR being defined but now is unconditionally defined. > >>> > >>> This will be useful reducing conditional code in other places that need > >>> to allocate from the module region (i.e., kprobes). > >>> > >>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > >>> --- > >>> v10: New to series > >>> --- > >>> arch/powerpc/include/asm/pgtable.h | 5 +++++ > >>> arch/powerpc/kernel/module.c | 5 +---- > >> > >> You probably also have changes to do in kernel/ptdump.c > >> > >> In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here. > >> > >>> 2 files changed, 6 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h > >>> index 4eed82172e33..014c2921f26a 100644 > >>> --- a/arch/powerpc/include/asm/pgtable.h > >>> +++ b/arch/powerpc/include/asm/pgtable.h > >>> @@ -167,6 +167,11 @@ struct seq_file; > >>> void arch_report_meminfo(struct seq_file *m); > >>> #endif /* CONFIG_PPC64 */ > >>> > >>> +#ifndef MODULES_VADDR > >>> +#define MODULES_VADDR VMALLOC_START > >>> +#define MODULES_END VMALLOC_END > >>> +#endif > >>> + > >>> #endif /* __ASSEMBLY__ */ > >>> > >>> #endif /* _ASM_POWERPC_PGTABLE_H */ > >>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c > >>> index a211b0253cdb..f1fb58389d58 100644 > >>> --- a/arch/powerpc/kernel/module.c > >>> +++ b/arch/powerpc/kernel/module.c > >>> @@ -14,6 +14,7 @@ > >>> #include <asm/firmware.h> > >>> #include <linux/sort.h> > >>> #include <asm/setup.h> > >>> +#include <linux/mm.h> > >>> > >>> static LIST_HEAD(module_bug_list); > >>> > >>> @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, > >>> return 0; > >>> } > >>> > >>> -#ifdef MODULES_VADDR > >>> void *module_alloc(unsigned long size) > >>> { > >>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); > >>> - > >> > >> The above check is needed somewhere, if you remove it from here you have to perform the check > >> somewhere else. > > > > This also introduces this warning: > > fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to > > false [-Wtautological-compare] > > 626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) { > > I might leave this patch out of this series and use an #ifdef for now > > and make this change separately as a follow up. > > x86/32 at least does the same (see > https://elixir.bootlin.com/linux/v5.12-rc8/source/arch/x86/include/asm/pgtable_32_areas.h#L47) > > They probably also get the warning, so I think would shouldn't bother. > One day someone will fix fs/proc/kcore.c , that's not a powerpc problem. Yeah you are right. I'll add the BUILD_BUG_ON() check to asm/task_size_32.h and keep the patch. > > > > >> > >>> return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, > >>> PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > >>> __builtin_return_address(0)); > >>> } > >>> -#endif > >>>
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h index 4eed82172e33..014c2921f26a 100644 --- a/arch/powerpc/include/asm/pgtable.h +++ b/arch/powerpc/include/asm/pgtable.h @@ -167,6 +167,11 @@ struct seq_file; void arch_report_meminfo(struct seq_file *m); #endif /* CONFIG_PPC64 */ +#ifndef MODULES_VADDR +#define MODULES_VADDR VMALLOC_START +#define MODULES_END VMALLOC_END +#endif + #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_PGTABLE_H */ diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c index a211b0253cdb..f1fb58389d58 100644 --- a/arch/powerpc/kernel/module.c +++ b/arch/powerpc/kernel/module.c @@ -14,6 +14,7 @@ #include <asm/firmware.h> #include <linux/sort.h> #include <asm/setup.h> +#include <linux/mm.h> static LIST_HEAD(module_bug_list); @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr, return 0; } -#ifdef MODULES_VADDR void *module_alloc(unsigned long size) { - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR); - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, __builtin_return_address(0)); } -#endif
If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and VMALLOC_END respectively. This reduces the need for special cases. For example, powerpc's module_alloc() was previously predicated on MODULES_VADDR being defined but now is unconditionally defined. This will be useful reducing conditional code in other places that need to allocate from the module region (i.e., kprobes). Signed-off-by: Jordan Niethe <jniethe5@gmail.com> --- v10: New to series --- arch/powerpc/include/asm/pgtable.h | 5 +++++ arch/powerpc/kernel/module.c | 5 +---- 2 files changed, 6 insertions(+), 4 deletions(-)