All of lore.kernel.org
 help / color / mirror / Atom feed
From: thunder.leizhen@huawei.com (Leizhen (ThunderTown))
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] arm64: fix flush_cache_range
Date: Tue, 24 May 2016 20:19:05 +0800	[thread overview]
Message-ID: <574446B9.8040105@huawei.com> (raw)
In-Reply-To: <20160524113727.GD25374@leverpostej>



On 2016/5/24 19:37, Mark Rutland wrote:
> On Tue, May 24, 2016 at 07:16:37PM +0800, Zhen Lei wrote:
>> When we ran mprotect04(a test case in LTP) infinitely, it would always
>> failed after a few seconds. The case can be described briefly that: copy
>> a empty function from code area into a new memory area(created by mmap),
>> then call mprotect to change the protection to PROT_EXEC. The syscall
>> sys_mprotect will finally invoke flush_cache_range, but this function
>> currently only invalid icache, the operation of flush dcache is missed.
> 
> In the LTP code I see powerpc-specific D-cache / I-cache synchronisation
> (i.e. d-cache cleaning followed by I-cache invalidation), so there
> appears to be some expectation of userspace maintenance. Hoever, there
> is no such ARM-specific I-cache maintenance.
But I see some other platforms have D-cache maintenance, like: arch/nios2/mm/cacheflush.c
And according to the name of flush_cache_range, it should do this, I judged. Otherwise,
mprotect04 will be failed on more platforms, it's easy to discover. Only PPC have specific
cache synchronization, maybe it meets some hardware limitation. It's impossible a programmer
fixed a common bug on only one platform but leave others unchanged.

> 
> It looks like the test may be missing I-cache maintenance regardless of
> the semantics of mprotect in this case.
> 
> I have not yet devled into flush_cache_range and how it is called.

SYSCALL_DEFINE3(mprotect ---> mprotect_fixup ---> change_protection ---> change_protection_range --> flush_cache_range

> 
> Thanks,
> Mark.
> 
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  arch/arm64/mm/flush.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
>> index dbd12ea..eda4124 100644
>> --- a/arch/arm64/mm/flush.c
>> +++ b/arch/arm64/mm/flush.c
>> @@ -31,7 +31,7 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start,
>>  		       unsigned long end)
>>  {
>>  	if (vma->vm_flags & VM_EXEC)
>> -		__flush_icache_all();
>> +		flush_icache_range(start, end);
>>  }
>>
>>  static void sync_icache_aliases(void *kaddr, unsigned long len)
>> --
>> 2.5.0
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> .
> 

WARNING: multiple messages have this Message-ID (diff)
From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Xinwei Hu <huxinwei@huawei.com>, Zefan Li <lizefan@huawei.com>,
	Hanjun Guo <guohanjun@huawei.com>,
	Tianhong Ding <dingtianhong@huawei.com>
Subject: Re: [PATCH 1/1] arm64: fix flush_cache_range
Date: Tue, 24 May 2016 20:19:05 +0800	[thread overview]
Message-ID: <574446B9.8040105@huawei.com> (raw)
In-Reply-To: <20160524113727.GD25374@leverpostej>



On 2016/5/24 19:37, Mark Rutland wrote:
> On Tue, May 24, 2016 at 07:16:37PM +0800, Zhen Lei wrote:
>> When we ran mprotect04(a test case in LTP) infinitely, it would always
>> failed after a few seconds. The case can be described briefly that: copy
>> a empty function from code area into a new memory area(created by mmap),
>> then call mprotect to change the protection to PROT_EXEC. The syscall
>> sys_mprotect will finally invoke flush_cache_range, but this function
>> currently only invalid icache, the operation of flush dcache is missed.
> 
> In the LTP code I see powerpc-specific D-cache / I-cache synchronisation
> (i.e. d-cache cleaning followed by I-cache invalidation), so there
> appears to be some expectation of userspace maintenance. Hoever, there
> is no such ARM-specific I-cache maintenance.
But I see some other platforms have D-cache maintenance, like: arch/nios2/mm/cacheflush.c
And according to the name of flush_cache_range, it should do this, I judged. Otherwise,
mprotect04 will be failed on more platforms, it's easy to discover. Only PPC have specific
cache synchronization, maybe it meets some hardware limitation. It's impossible a programmer
fixed a common bug on only one platform but leave others unchanged.

> 
> It looks like the test may be missing I-cache maintenance regardless of
> the semantics of mprotect in this case.
> 
> I have not yet devled into flush_cache_range and how it is called.

SYSCALL_DEFINE3(mprotect ---> mprotect_fixup ---> change_protection ---> change_protection_range --> flush_cache_range

> 
> Thanks,
> Mark.
> 
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  arch/arm64/mm/flush.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/flush.c b/arch/arm64/mm/flush.c
>> index dbd12ea..eda4124 100644
>> --- a/arch/arm64/mm/flush.c
>> +++ b/arch/arm64/mm/flush.c
>> @@ -31,7 +31,7 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start,
>>  		       unsigned long end)
>>  {
>>  	if (vma->vm_flags & VM_EXEC)
>> -		__flush_icache_all();
>> +		flush_icache_range(start, end);
>>  }
>>
>>  static void sync_icache_aliases(void *kaddr, unsigned long len)
>> --
>> 2.5.0
>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 
> .
> 

  reply	other threads:[~2016-05-24 12:19 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24 11:16 [PATCH 1/1] arm64: fix flush_cache_range Zhen Lei
2016-05-24 11:16 ` Zhen Lei
2016-05-24 11:37 ` Mark Rutland
2016-05-24 11:37   ` Mark Rutland
2016-05-24 12:19   ` Leizhen (ThunderTown) [this message]
2016-05-24 12:19     ` Leizhen (ThunderTown)
2016-05-24 13:02     ` Catalin Marinas
2016-05-24 13:02       ` Catalin Marinas
2016-05-25  1:20       ` Leizhen (ThunderTown)
2016-05-25  3:36         ` Leizhen (ThunderTown)
2016-05-25  3:36           ` Leizhen (ThunderTown)
2016-05-25 10:50           ` Catalin Marinas
2016-05-25 10:50             ` Catalin Marinas
2016-05-26  3:40             ` Leizhen (ThunderTown)
2016-05-26  3:40               ` Leizhen (ThunderTown)
2016-05-26 11:46             ` Leizhen (ThunderTown)
2016-05-26 11:46               ` Leizhen (ThunderTown)
2016-05-26 12:04               ` Russell King - ARM Linux
2016-05-26 12:04                 ` Russell King - ARM Linux
2016-05-26 16:36               ` Catalin Marinas
2016-05-26 16:36                 ` Catalin Marinas
2016-05-24 15:12     ` Mark Rutland
2016-05-24 15:12       ` Mark Rutland
2016-05-25 15:22       ` Catalin Marinas
2016-05-25 15:22         ` Catalin Marinas
2016-05-25 17:27         ` Russell King - ARM Linux
2016-05-25 17:27           ` Russell King - ARM Linux
2016-05-26 16:44           ` Catalin Marinas
2016-05-26 16:44             ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=574446B9.8040105@huawei.com \
    --to=thunder.leizhen@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.