When enable KASLR, livepatch will adjust old_addr of changed
function accordingly. So do the same thing for reloc.
[PATCH v1] https://lkml.org/lkml/2015/11/4/91
Reported-by: Cyril B. <[email protected]>
Signed-off-by: Zhou Chengming <[email protected]>
---
kernel/livepatch/core.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 6e53441..db545cb 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -294,6 +294,12 @@ static int klp_write_object_relocations(struct module *pmod,
for (reloc = obj->relocs; reloc->name; reloc++) {
if (!klp_is_module(obj)) {
+
+#if defined(CONFIG_RANDOMIZE_BASE)
+ /* If KASLR has been enabled, adjust old value accordingly */
+ if (kaslr_enabled())
+ reloc->val += kaslr_offset();
+#endif
ret = klp_verify_vmlinux_symbol(reloc->name,
reloc->val);
if (ret)
--
1.7.7
On Fri, Nov 06, 2015 at 02:25:00PM +0800, Zhou Chengming wrote:
> When enable KASLR, livepatch will adjust old_addr of changed
> function accordingly. So do the same thing for reloc.
>
> [PATCH v1] https://lkml.org/lkml/2015/11/4/91
>
> Reported-by: Cyril B. <[email protected]>
> Signed-off-by: Zhou Chengming <[email protected]>
> ---
> kernel/livepatch/core.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 6e53441..db545cb 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -294,6 +294,12 @@ static int klp_write_object_relocations(struct module *pmod,
>
> for (reloc = obj->relocs; reloc->name; reloc++) {
> if (!klp_is_module(obj)) {
> +
> +#if defined(CONFIG_RANDOMIZE_BASE)
> + /* If KASLR has been enabled, adjust old value accordingly */
> + if (kaslr_enabled())
> + reloc->val += kaslr_offset();
> +#endif
> ret = klp_verify_vmlinux_symbol(reloc->name,
> reloc->val);
> if (ret)
Zhou, thanks a lot for this fix.
Generally I think this patch is fine. However, Chris J Arges is working
on another patch[*] which may get rid of reloc->val as an input and make
this patch obsolete.
So, assuming Chris's patch eventually gets accepted, I don't see a need
for this one unless anybody wants it as a bug fix for 4.4.
[*] https://lkml.kernel.org/r/[email protected]
--
Josh
On 11/10/15 at 08:07am, Josh Poimboeuf wrote:
> On Fri, Nov 06, 2015 at 02:25:00PM +0800, Zhou Chengming wrote:
> > When enable KASLR, livepatch will adjust old_addr of changed
> > function accordingly. So do the same thing for reloc.
> >
> > +
> > +#if defined(CONFIG_RANDOMIZE_BASE)
> > + /* If KASLR has been enabled, adjust old value accordingly */
> > + if (kaslr_enabled())
> > + reloc->val += kaslr_offset();
> > +#endif
> > ret = klp_verify_vmlinux_symbol(reloc->name,
> > reloc->val);
> > if (ret)
>
> Zhou, thanks a lot for this fix.
>
> Generally I think this patch is fine. However, Chris J Arges is working
> on another patch[*] which may get rid of reloc->val as an input and make
> this patch obsolete.
>
> So, assuming Chris's patch eventually gets accepted, I don't see a need
> for this one unless anybody wants it as a bug fix for 4.4.
>
> [*] https://lkml.kernel.org/r/[email protected]
>
Hi, Josh.
I think Jessica Yu is working on the relative patchset which will
offload relocation logical to the module loader.
Thanks
Minfei
On Wed, Nov 11, 2015 at 04:46:48PM +0800, Minfei Huang wrote:
> On 11/10/15 at 08:07am, Josh Poimboeuf wrote:
> > On Fri, Nov 06, 2015 at 02:25:00PM +0800, Zhou Chengming wrote:
> > > When enable KASLR, livepatch will adjust old_addr of changed
> > > function accordingly. So do the same thing for reloc.
> > >
> > > +
> > > +#if defined(CONFIG_RANDOMIZE_BASE)
> > > + /* If KASLR has been enabled, adjust old value accordingly */
> > > + if (kaslr_enabled())
> > > + reloc->val += kaslr_offset();
> > > +#endif
> > > ret = klp_verify_vmlinux_symbol(reloc->name,
> > > reloc->val);
> > > if (ret)
> >
> > Zhou, thanks a lot for this fix.
> >
> > Generally I think this patch is fine. However, Chris J Arges is working
> > on another patch[*] which may get rid of reloc->val as an input and make
> > this patch obsolete.
> >
> > So, assuming Chris's patch eventually gets accepted, I don't see a need
> > for this one unless anybody wants it as a bug fix for 4.4.
> >
> > [*] https://lkml.kernel.org/r/[email protected]
> >
>
> Hi, Josh.
>
> I think Jessica Yu is working on the relative patchset which will
> offload relocation logical to the module loader.
Yeah, Jessica's patch set does move the relocation logic to the module
loader, and it does result in a rewrite of this code. However, it still
uses an address-based symbol addressing scheme, so it doesn't resolve
this particular issue.
Chris's patch changes the symbol addressing scheme from address-based to
sympos-based. To be consistent, I think it should do that for both
function addresses and relocation symbol addresses. Then that would fix
this issue and make Zhou's patch obsolete.
If Chris's smaller patch is merged before Jessica's bigger patch set,
Jessica's patches can be rebased on top of Chris's to keep the new
sympos-based addressing scheme.
Or vice versa: if Jessica's patches are merged first, then Chris's can
be rebased. Either way, Chris's patches will obsolete this one.
--
Josh
On Wed, 11 Nov 2015, Josh Poimboeuf wrote:
> If Chris's smaller patch is merged before Jessica's bigger patch set,
> Jessica's patches can be rebased on top of Chris's to keep the new
> sympos-based addressing scheme.
>
> Or vice versa: if Jessica's patches are merged first, then Chris's can
> be rebased. Either way, Chris's patches will obsolete this one.
My current plan is to take Zhou's simple kASLR offset fix for 4.4 still
(it's my fault that I overlooked this when putting kASLR bits in place
anyway), and we'll see what ends up being merged for 4.5+.
Thanks,
--
Jiri Kosina
SUSE Labs
On Wed, Nov 11, 2015 at 05:19:58PM +0100, Jiri Kosina wrote:
> On Wed, 11 Nov 2015, Josh Poimboeuf wrote:
>
> > If Chris's smaller patch is merged before Jessica's bigger patch set,
> > Jessica's patches can be rebased on top of Chris's to keep the new
> > sympos-based addressing scheme.
> >
> > Or vice versa: if Jessica's patches are merged first, then Chris's can
> > be rebased. Either way, Chris's patches will obsolete this one.
>
> My current plan is to take Zhou's simple kASLR offset fix for 4.4 still
> (it's my fault that I overlooked this when putting kASLR bits in place
> anyway), and we'll see what ends up being merged for 4.5+.
Ok, sounds fine to me.
--
Josh
On Fri, Nov 06, 2015 at 02:25:00PM +0800, Zhou Chengming wrote:
> When enable KASLR, livepatch will adjust old_addr of changed
> function accordingly. So do the same thing for reloc.
>
> [PATCH v1] https://lkml.org/lkml/2015/11/4/91
>
> Reported-by: Cyril B. <[email protected]>
> Signed-off-by: Zhou Chengming <[email protected]>
> ---
> kernel/livepatch/core.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 6e53441..db545cb 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -294,6 +294,12 @@ static int klp_write_object_relocations(struct module *pmod,
>
> for (reloc = obj->relocs; reloc->name; reloc++) {
> if (!klp_is_module(obj)) {
> +
> +#if defined(CONFIG_RANDOMIZE_BASE)
> + /* If KASLR has been enabled, adjust old value accordingly */
> + if (kaslr_enabled())
> + reloc->val += kaslr_offset();
> +#endif
> ret = klp_verify_vmlinux_symbol(reloc->name,
> reloc->val);
> if (ret)
Acked-by: Josh Poimboeuf <[email protected]>
On Fri, 6 Nov 2015, Zhou Chengming wrote:
> When enable KASLR, livepatch will adjust old_addr of changed
> function accordingly. So do the same thing for reloc.
>
> [PATCH v1] https://lkml.org/lkml/2015/11/4/91
>
> Reported-by: Cyril B. <[email protected]>
> Signed-off-by: Zhou Chengming <[email protected]>
I have made the changelog and patch title a little bit more verbose and
applied to for-4.4/upstream-fixes branch.
Thanks,
--
Jiri Kosina
SUSE Labs