Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1263766 > unrolled thread

[PATCH v2] livepatch: x86: bugfix about kASLR

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] livepatch: x86: bugfix about kASLR Zhou Chengming <zhouchengming1@huawei.com> - 2015-11-06 07:40 +0100
    Re: [PATCH v2] livepatch: x86: bugfix about kASLR Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-10 15:10 +0100
      Re: [PATCH v2] livepatch: x86: bugfix about kASLR Minfei Huang <mnfhuang@gmail.com> - 2015-11-11 09:50 +0100
        Re: [PATCH v2] livepatch: x86: bugfix about kASLR Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-11 17:20 +0100
          Re: [PATCH v2] livepatch: x86: bugfix about kASLR Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-11 17:30 +0100
          Re: [PATCH v2] livepatch: x86: bugfix about kASLR Jiri Kosina <jikos@kernel.org> - 2015-11-11 17:30 +0100
    Re: [PATCH v2] livepatch: x86: bugfix about kASLR Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-11 17:30 +0100
    Re: [PATCH v2] livepatch: x86: bugfix about kASLR Jiri Kosina <jikos@kernel.org> - 2015-11-11 17:50 +0100

#1263766 — [PATCH v2] livepatch: x86: bugfix about kASLR

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. <cbay@alwaysdata.com>
Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1266520

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. <cbay@alwaysdata.com>
> Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
> ---
>  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/1447085770-11729-1-git-send-email-chris.j.arges@canonical.com

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267041

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/1447085770-11729-1-git-send-email-chris.j.arges@canonical.com
> 

Hi, Josh.

I think Jessica Yu is working on the relative patchset which will
offload relocation logical to the module loader.

Thanks
Minfei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267287

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/1447085770-11729-1-git-send-email-chris.j.arges@canonical.com
> > 
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267289

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267293

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267290

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. <cbay@alwaysdata.com>
> Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
> ---
>  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 <jpoimboe@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267313

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. <cbay@alwaysdata.com>
> Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web