[PATCH] livepatch: x86: bugfix about kASLR

[PATCH] livepatch: x86: bugfix about kASLR

[Zhou Chengming]

When enable KASLR, func->old_addr will be set to zero
and livepatch will find the right old address.
But for reloc, livepatch just verify it using reloc->val
(old addr from user), so verify failed and report
"kernel mismatch" error.

Reported-by: Cyril B. <cbay@alwaysdata.com>
Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
---
 kernel/livepatch/core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 53196e2..c8885c6 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -311,7 +311,12 @@ static int klp_write_object_relocations(struct module *pmod,
 		return -EINVAL;
 
 	for (reloc = obj->relocs; reloc->name; reloc++) {
-		if (!klp_is_module(obj)) {
+
+#if defined(CONFIG_RANDOMIZE_BASE)
+		/* KASLR is enabled, disregard old_addr from user */
+		reloc->val = 0;
+#endif
+		if (reloc->val && !klp_is_module(obj)) {
 			ret = klp_verify_vmlinux_symbol(reloc->name,
 							reloc->val);
 			if (ret)
-- 
1.7.7

Re: [PATCH] livepatch: x86: bugfix about kASLR

[Jiri Kosina]

On Wed, 4 Nov 2015, Zhou Chengming wrote:

> When enable KASLR, func->old_addr will be set to zero
> and livepatch will find the right old address.
> But for reloc, livepatch just verify it using reloc->val
> (old addr from user), so verify failed and report
> "kernel mismatch" error.
> 
> Reported-by: Cyril B. <cbay@alwaysdata.com>
> Signed-off-by: Zhou Chengming <zhouchengming1@huawei.com>
> ---
>  kernel/livepatch/core.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 53196e2..c8885c6 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -311,7 +311,12 @@ static int klp_write_object_relocations(struct module *pmod,
>  		return -EINVAL;
>  
>  	for (reloc = obj->relocs; reloc->name; reloc++) {
> -		if (!klp_is_module(obj)) {
> +
> +#if defined(CONFIG_RANDOMIZE_BASE)

CONFIG_RANDOMIZE_BASE being enabled by itself doesn't directly imply that 
kASLR has been actually really enabled during runtime. You need 
to check kaslr_enabled() as well.

> +		/* KASLR is enabled, disregard old_addr from user */
> +		reloc->val = 0;

Is there a reason why to discard it completely? Adding kaslr_offset() 
should give us the correct address, right?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] livepatch: x86: bugfix about kASLR

[zhouchengming]

On 2015/11/4 17:44, Jiri Kosina wrote:
> On Wed, 4 Nov 2015, Zhou Chengming wrote:
>
>> When enable KASLR, func->old_addr will be set to zero
>> and livepatch will find the right old address.
>> But for reloc, livepatch just verify it using reloc->val
>> (old addr from user), so verify failed and report
>> "kernel mismatch" error.
>>
>> Reported-by: Cyril B.<cbay@alwaysdata.com>
>> Signed-off-by: Zhou Chengming<zhouchengming1@huawei.com>
>> ---
>>   kernel/livepatch/core.c |    7 ++++++-
>>   1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index 53196e2..c8885c6 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -311,7 +311,12 @@ static int klp_write_object_relocations(struct module *pmod,
>>   		return -EINVAL;
>>
>>   	for (reloc = obj->relocs; reloc->name; reloc++) {
>> -		if (!klp_is_module(obj)) {
>> +
>> +#if defined(CONFIG_RANDOMIZE_BASE)
> CONFIG_RANDOMIZE_BASE being enabled by itself doesn't directly imply that
> kASLR has been actually really enabled during runtime. You need
> to check kaslr_enabled() as well.

Right.

>> +		/* KASLR is enabled, disregard old_addr from user */
>> +		reloc->val = 0;
> Is there a reason why to discard it completely? Adding kaslr_offset()
> should give us the correct address, right?
>
> Thanks,
>

Yes, it's definitely better.

Thanks

Re: [PATCH] livepatch: x86: bugfix about kASLR

[Cyril B.]

Zhou Chengming wrote:
> When enable KASLR, func->old_addr will be set to zero
> and livepatch will find the right old address.
> But for reloc, livepatch just verify it using reloc->val
> (old addr from user), so verify failed and report
> "kernel mismatch" error.
>
> Reported-by: Cyril B.<cbay@alwaysdata.com>
> Signed-off-by: Zhou Chengming<zhouchengming1@huawei.com>
> ---
>   kernel/livepatch/core.c |    7 ++++++-
>   1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 53196e2..c8885c6 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -311,7 +311,12 @@ static int klp_write_object_relocations(struct module *pmod,
>   		return -EINVAL;
>
>   	for (reloc = obj->relocs; reloc->name; reloc++) {
> -		if (!klp_is_module(obj)) {
> +
> +#if defined(CONFIG_RANDOMIZE_BASE)
> +		/* KASLR is enabled, disregard old_addr from user */
> +		reloc->val = 0;
> +#endif
> +		if (reloc->val&&  !klp_is_module(obj)) {
>   			ret = klp_verify_vmlinux_symbol(reloc->name,
>   							reloc->val);
>   			if (ret)

I still get the 'symbol not found in symbol table' error with that patch 
(on top of Linux 4.3).

Cyril