2015-02-25 22:15:09

by Laura Abbott

[permalink] [raw]
Subject: [PATCH 0/3] CONFIG_DEBUG_SET_MODULE_RONX fixups

Hi,

CONFIG_DEBUG_SET_MODULE_RONX is currently non-functional on arm and arm64
because of changes in behavior of is_module_addr. This series fixes
both arm and arm64 to work correctly and corrects a minor bug
related to section alignment in modules.

Laura Abbott (3):
arm64: Don't use is_module_addr in setting page attributes
arm: Don't use is_module_addr in setting page attributes
kernel/module.c: Update debug alignment after symtable generation

arch/arm/mm/pageattr.c | 5 ++++-
arch/arm64/mm/pageattr.c | 5 ++++-
kernel/module.c | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)

--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project


2015-02-25 22:15:11

by Laura Abbott

[permalink] [raw]
Subject: [PATCH 1/3] arm64: Don't use is_module_addr in setting page attributes


The set_memory_* functions currently only support module
addresses. The addresses are validated using is_module_addr.
That function is special though and relies on internal state
in the module subsystem to work properly. At the time of
module initialization and calling set_memory_*, it's too early
for is_module_addr to work properly so it always returns
false. Rather than be subject to the whims of the module state,
just bounds check against the module virtual address range.

Signed-off-by: Laura Abbott <[email protected]>
---
arch/arm64/mm/pageattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index bb0ea94..1d3ec3d 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -51,7 +51,10 @@ static int change_memory_common(unsigned long addr, int numpages,
WARN_ON_ONCE(1);
}

- if (!is_module_address(start) || !is_module_address(end - 1))
+ if (start < MODULES_VADDR || start >= MODULES_END)
+ return -EINVAL;
+
+ if (end < MODULES_VADDR || end >= MODULES_END)
return -EINVAL;

data.set_mask = set_mask;
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

2015-02-25 22:15:13

by Laura Abbott

[permalink] [raw]
Subject: [PATCH 2/3] arm: Don't use is_module_addr in setting page attributes


The set_memory_* functions currently only support module
addresses. The addresses are validated using is_module_addr.
That function is special though and relies on internal state
in the module subsystem to work properly. At the time of
module initialization and calling set_memory_*, it's too early
for is_module_addr to work properly so it always returns
false. Rather than be subject to the whims of the module state,
just bounds check against the module virtual address range.

Signed-off-by: Laura Abbott <[email protected]>
---
arch/arm/mm/pageattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
index 004e35c..cf30daf 100644
--- a/arch/arm/mm/pageattr.c
+++ b/arch/arm/mm/pageattr.c
@@ -49,7 +49,10 @@ static int change_memory_common(unsigned long addr, int numpages,
WARN_ON_ONCE(1);
}

- if (!is_module_address(start) || !is_module_address(end - 1))
+ if (start < MODULES_VADDR || start >= MODULES_END)
+ return -EINVAL;
+
+ if (end < MODULES_VADDR || start >= MODULES_END)
return -EINVAL;

data.set_mask = set_mask;
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

2015-02-25 22:16:03

by Laura Abbott

[permalink] [raw]
Subject: [PATCH 3/3] kernel/module.c: Update debug alignment after symtable generation


When CONFIG_DEBUG_SET_MODULE_RONX is enabled, the sizes of
module sections are aligned up so appropriate permissions can
be applied. Adjusting for the symbol table may cause them to
become unaligned. Make sure to re-align the sizes afterward.

Signed-off-by: Laura Abbott <[email protected]>
---
kernel/module.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index b34813f..cc93cf6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2313,11 +2313,13 @@ static void layout_symtab(struct module *mod, struct load_info *info)
info->symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1);
info->stroffs = mod->core_size = info->symoffs + ndst * sizeof(Elf_Sym);
mod->core_size += strtab_size;
+ mod->core_size = debug_align(mod->core_size);

/* Put string table section at end of init part of module. */
strsect->sh_flags |= SHF_ALLOC;
strsect->sh_entsize = get_offset(mod, &mod->init_size, strsect,
info->index.str) | INIT_OFFSET_MASK;
+ mod->init_size = debug_align(mod->init_size);
pr_debug("\t%s\n", info->secstrings + strsect->sh_name);
}

--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

2015-02-25 22:17:57

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 0/3] CONFIG_DEBUG_SET_MODULE_RONX fixups

On Wed, Feb 25, 2015 at 2:14 PM, Laura Abbott <[email protected]> wrote:
> Hi,
>
> CONFIG_DEBUG_SET_MODULE_RONX is currently non-functional on arm and arm64
> because of changes in behavior of is_module_addr. This series fixes
> both arm and arm64 to work correctly and corrects a minor bug
> related to section alignment in modules.
>
> Laura Abbott (3):
> arm64: Don't use is_module_addr in setting page attributes
> arm: Don't use is_module_addr in setting page attributes
> kernel/module.c: Update debug alignment after symtable generation
>
> arch/arm/mm/pageattr.c | 5 ++++-
> arch/arm64/mm/pageattr.c | 5 ++++-
> kernel/module.c | 2 ++
> 3 files changed, 10 insertions(+), 2 deletions(-)

Thanks for fixing this!

Out of curiosity, which change broke DEBUG_SET_MODULE_RONX ? (i.e.
does this need a CC: stable, and if so, through which release?)

Regardless, consider them:

Reviewed-by: Kees Cook <[email protected]>

Thanks!

-Kees

--
Kees Cook
Chrome OS Security

2015-02-26 03:11:48

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 3/3] kernel/module.c: Update debug alignment after symtable generation

Laura Abbott <[email protected]> writes:
> When CONFIG_DEBUG_SET_MODULE_RONX is enabled, the sizes of
> module sections are aligned up so appropriate permissions can
> be applied. Adjusting for the symbol table may cause them to
> become unaligned. Make sure to re-align the sizes afterward.
>
> Signed-off-by: Laura Abbott <[email protected]>

Acked-by: Rusty Russell <[email protected]>

This won't clash with anything I'm planning, so happy for this to go in
through the arm trees. CC:stable should be fine if you want too.

Thanks,
Rusty.

> ---
> kernel/module.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index b34813f..cc93cf6 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2313,11 +2313,13 @@ static void layout_symtab(struct module *mod, struct load_info *info)
> info->symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1);
> info->stroffs = mod->core_size = info->symoffs + ndst * sizeof(Elf_Sym);
> mod->core_size += strtab_size;
> + mod->core_size = debug_align(mod->core_size);
>
> /* Put string table section at end of init part of module. */
> strsect->sh_flags |= SHF_ALLOC;
> strsect->sh_entsize = get_offset(mod, &mod->init_size, strsect,
> info->index.str) | INIT_OFFSET_MASK;
> + mod->init_size = debug_align(mod->init_size);
> pr_debug("\t%s\n", info->secstrings + strsect->sh_name);
> }
>
> --
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/