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


Groups > linux.kernel > #1306896 > unrolled thread

Re: Have any influence on set_memory_** about below patch ??

Started byXishi Qiu <qiuxishi@huawei.com>
First post2016-01-12 02:30 +0100
Last post2016-01-14 14:50 +0100
Articles 12 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-12 02:30 +0100
    Re: Have any influence on set_memory_** about below patch ?? Mark Rutland <mark.rutland@arm.com> - 2016-01-12 12:20 +0100
      Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-13 05:20 +0100
        Re: Have any influence on set_memory_** about below patch ?? Mark Rutland <mark.rutland@arm.com> - 2016-01-13 12:30 +0100
      Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-13 06:10 +0100
        Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-13 07:40 +0100
        Re: Have any influence on set_memory_** about below patch ?? Mark Rutland <mark.rutland@arm.com> - 2016-01-13 12:30 +0100
      Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-13 11:40 +0100
        Re: Have any influence on set_memory_** about below patch ?? Mark Rutland <mark.rutland@arm.com> - 2016-01-13 12:20 +0100
          Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-14 13:40 +0100
            Re: Have any influence on set_memory_** about below patch ?? Xishi Qiu <qiuxishi@huawei.com> - 2016-01-14 14:10 +0100
              Re: Have any influence on set_memory_** about below patch ?? Mark Rutland <mark.rutland@arm.com> - 2016-01-14 14:50 +0100

#1306896 — Re: Have any influence on set_memory_** about below patch ??

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-12 02:30 +0100
SubjectRe: Have any influence on set_memory_** about below patch ??
Message-ID<qPVWy-2fD-19@gated-at.bofh.it>
On 2016/1/11 21:31, Mark Rutland wrote:

> Hi,
> 
> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
>>
>> http://www.spinics.net/lists/arm-kernel/msg472090.html
>>
>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
>> and merging wiil produce confilct in the liner mapping area. Based on the
>> situation, Assume that set up page table in 4kb page table way in the liner
>> mapping area, Does the set_memroy_** will work without any conplict??
> 
> I'm not sure I understand the question.
> 
> I'm also not a fan of responding to off-list queries as information gets
> lost.
> 
> Please ask your question on the mailing list. I am more than happy to
> respond there.
> 
> Thanks,
> Mark.
> 

Hi Mark,

In your patch it said "The presence of conflicting TLB entries may result in
a variety of behaviours detrimental to the system " and "but this(break-before-make
approach) cannot work for modifications to the swapper page tables that cover the
kernel text and data."

I'm not quite understand this, why the direct mapping can't work?
flush tlb can't resolve it?

I find x86 does not have this limit. e.g. set_memory_r*.

Thanks,
Xishi Qiu

> .
> 

[toc] | [next] | [standalone]


#1307281

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-12 12:20 +0100
Message-ID<qQ59w-8S-29@gated-at.bofh.it>
In reply to#1306896
On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
> On 2016/1/11 21:31, Mark Rutland wrote:
> 
> > Hi,
> > 
> > On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
> >>
> >> http://www.spinics.net/lists/arm-kernel/msg472090.html
> >>
> >> Hi, Can I ask you a question? Say, This patch tells that the section spilting
> >> and merging wiil produce confilct in the liner mapping area. Based on the
> >> situation, Assume that set up page table in 4kb page table way in the liner
> >> mapping area, Does the set_memroy_** will work without any conplict??
> > 
> > I'm not sure I understand the question.
> > 
> > I'm also not a fan of responding to off-list queries as information gets
> > lost.
> > 
> > Please ask your question on the mailing list. I am more than happy to
> > respond there.
> > 
> > Thanks,
> > Mark.
> > 
> 
> Hi Mark,
> 
> In your patch it said "The presence of conflicting TLB entries may result in
> a variety of behaviours detrimental to the system " and "but this(break-before-make
> approach) cannot work for modifications to the swapper page tables that cover the
> kernel text and data."
> 
> I'm not quite understand this, why the direct mapping can't work?

The problem is that the TLB hardware can operate asynchronously to the
rest of the CPU. At any point in time, for any reason, it can decide to
destroy TLB entries, to allocate new ones, or to perform a walk based on
the existing contents of the TLB.

When the TLB contains conflicting entries, TLB lookups may result in TLB
conflict aborts, or may return an "amalgamation" of the conflicting
entries (e.g. you could get an erroneous output address).

The direct mapping is in active use (and hence live in TLBs). Modifying
it without break-before-make (BBM) risks the allocation of conflicting
TLB entries. Modifying it with BBM risks unmapping the portion of the
kernel performing the modification, resulting in an unrecoverable abort.

> flush tlb can't resolve it?

Flushing the TLB doesn't help because the page table update, TLB
invalidate, and corresponding barrier(s) are separate operations. The
TLB can allocate or destroy entries at any point during the sequence.

For example, without BBM a page table update would look something like:

1)	str	<newpte>, [<*pte>]
2)	dsb	ish
3)	tlbi	vmalle1is
4)	dsb	ish
5)	isb

After step 1, the new pte value may become visible to the TLBs, and the
TLBs may allocate a new entry for it. Until step 4 completes, this entry
may remain active in the TLB, and may conflict with an existing entry.

If that entry covers the kernel text for steps 2-5, executing the
sequence may result in an unrecoverable TLB conflict abort, or some
other behaviour resulting from an amalgamated TLB, e.g. the I-cache
might fetch instructions from the wrong address such that steps 2-5
cannot be executed.

If the kernel doesn't explicitly access the address covered by that pte,
there may still be a problem. The TLB may perform an internal lookup
when performing a page table walk, and could then use an erroneous
result to continue the walk, resulting in a variety of potential issues
(e.g. reading from an MMIO peripheral register).

BBM avoids the conflict, but as that would mean kernel text and/or data
would be unmapped, you can't execute the code to finish the update.

> I find x86 does not have this limit. e.g. set_memory_r*.

I don't know much about x86; it's probably worth asking the x86 guys
about that. It may be that the x86 architecture requires that a conflict
or amalgamation is never visible to software, or it could be that
contemporary implementations happen to provide that property.

Thanks,
Mark.

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


#1308030

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-13 05:20 +0100
Message-ID<qQl4C-2Je-1@gated-at.bofh.it>
In reply to#1307281
On 2016/1/12 19:15, Mark Rutland wrote:

> On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
>> On 2016/1/11 21:31, Mark Rutland wrote:
>>
>>> Hi,
>>>
>>> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
>>>>
>>>> http://www.spinics.net/lists/arm-kernel/msg472090.html
>>>>
>>>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
>>>> and merging wiil produce confilct in the liner mapping area. Based on the
>>>> situation, Assume that set up page table in 4kb page table way in the liner
>>>> mapping area, Does the set_memroy_** will work without any conplict??
>>>
>>> I'm not sure I understand the question.
>>>
>>> I'm also not a fan of responding to off-list queries as information gets
>>> lost.
>>>
>>> Please ask your question on the mailing list. I am more than happy to
>>> respond there.
>>>
>>> Thanks,
>>> Mark.
>>>
>>
>> Hi Mark,
>>
>> In your patch it said "The presence of conflicting TLB entries may result in
>> a variety of behaviours detrimental to the system " and "but this(break-before-make
>> approach) cannot work for modifications to the swapper page tables that cover the
>> kernel text and data."
>>
>> I'm not quite understand this, why the direct mapping can't work?
> 
> The problem is that the TLB hardware can operate asynchronously to the
> rest of the CPU. At any point in time, for any reason, it can decide to
> destroy TLB entries, to allocate new ones, or to perform a walk based on
> the existing contents of the TLB.
> 
> When the TLB contains conflicting entries, TLB lookups may result in TLB
> conflict aborts, or may return an "amalgamation" of the conflicting
> entries (e.g. you could get an erroneous output address).
> 
> The direct mapping is in active use (and hence live in TLBs). Modifying
> it without break-before-make (BBM) risks the allocation of conflicting
> TLB entries. Modifying it with BBM risks unmapping the portion of the
> kernel performing the modification, resulting in an unrecoverable abort.
> 
>> flush tlb can't resolve it?
> 
> Flushing the TLB doesn't help because the page table update, TLB
> invalidate, and corresponding barrier(s) are separate operations. The
> TLB can allocate or destroy entries at any point during the sequence.
> 
> For example, without BBM a page table update would look something like:
> 
> 1)	str	<newpte>, [<*pte>]
> 2)	dsb	ish
> 3)	tlbi	vmalle1is
> 4)	dsb	ish
> 5)	isb
> 
> After step 1, the new pte value may become visible to the TLBs, and the
> TLBs may allocate a new entry for it. Until step 4 completes, this entry
> may remain active in the TLB, and may conflict with an existing entry.
> 
> If that entry covers the kernel text for steps 2-5, executing the
> sequence may result in an unrecoverable TLB conflict abort, or some
> other behaviour resulting from an amalgamated TLB, e.g. the I-cache
> might fetch instructions from the wrong address such that steps 2-5
> cannot be executed.
> 
> If the kernel doesn't explicitly access the address covered by that pte,
> there may still be a problem. The TLB may perform an internal lookup
> when performing a page table walk, and could then use an erroneous
> result to continue the walk, resulting in a variety of potential issues
> (e.g. reading from an MMIO peripheral register).
> 
> BBM avoids the conflict, but as that would mean kernel text and/or data
> would be unmapped, you can't execute the code to finish the update.
> 
>> I find x86 does not have this limit. e.g. set_memory_r*.
> 
> I don't know much about x86; it's probably worth asking the x86 guys
> about that. It may be that the x86 architecture requires that a conflict
> or amalgamation is never visible to software, or it could be that
> contemporary implementations happen to provide that property.
> 
> Thanks,
> Mark.
> 

Hi Mark,

Thank you for your reply, I find this code in /arch/arm64/mm/mmu.c

...
#ifdef CONFIG_DEBUG_RODATA
void mark_rodata_ro(void)
{
	create_mapping_late(__pa(_stext), (unsigned long)_stext,
				(unsigned long)_etext - (unsigned long)_stext,
				PAGE_KERNEL_EXEC | PTE_RDONLY);

}
#endif
...

So does it also have this problem?

Thanks,
Xishi Qiu

> .
> 

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


#1308309

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-13 12:30 +0100
Message-ID<qQrMK-7nW-27@gated-at.bofh.it>
In reply to#1308030
On Wed, Jan 13, 2016 at 12:10:29PM +0800, Xishi Qiu wrote:
> On 2016/1/12 19:15, Mark Rutland wrote:
> 
> > On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
> >> On 2016/1/11 21:31, Mark Rutland wrote:
> >>
> >>> Hi,
> >>>
> >>> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
> >>>>
> >>>> http://www.spinics.net/lists/arm-kernel/msg472090.html
> >>>>
> >>>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
> >>>> and merging wiil produce confilct in the liner mapping area. Based on the
> >>>> situation, Assume that set up page table in 4kb page table way in the liner
> >>>> mapping area, Does the set_memroy_** will work without any conplict??
> >>>
> >>> I'm not sure I understand the question.
> >>>
> >>> I'm also not a fan of responding to off-list queries as information gets
> >>> lost.
> >>>
> >>> Please ask your question on the mailing list. I am more than happy to
> >>> respond there.
> >>>
> >>> Thanks,
> >>> Mark.
> >>>
> >>
> >> Hi Mark,
> >>
> >> In your patch it said "The presence of conflicting TLB entries may result in
> >> a variety of behaviours detrimental to the system " and "but this(break-before-make
> >> approach) cannot work for modifications to the swapper page tables that cover the
> >> kernel text and data."
> >>
> >> I'm not quite understand this, why the direct mapping can't work?
> > 
> > The problem is that the TLB hardware can operate asynchronously to the
> > rest of the CPU. At any point in time, for any reason, it can decide to
> > destroy TLB entries, to allocate new ones, or to perform a walk based on
> > the existing contents of the TLB.
> > 
> > When the TLB contains conflicting entries, TLB lookups may result in TLB
> > conflict aborts, or may return an "amalgamation" of the conflicting
> > entries (e.g. you could get an erroneous output address).
> > 
> > The direct mapping is in active use (and hence live in TLBs). Modifying
> > it without break-before-make (BBM) risks the allocation of conflicting
> > TLB entries. Modifying it with BBM risks unmapping the portion of the
> > kernel performing the modification, resulting in an unrecoverable abort.
> > 
> >> flush tlb can't resolve it?
> > 
> > Flushing the TLB doesn't help because the page table update, TLB
> > invalidate, and corresponding barrier(s) are separate operations. The
> > TLB can allocate or destroy entries at any point during the sequence.
> > 
> > For example, without BBM a page table update would look something like:
> > 
> > 1)	str	<newpte>, [<*pte>]
> > 2)	dsb	ish
> > 3)	tlbi	vmalle1is
> > 4)	dsb	ish
> > 5)	isb
> > 
> > After step 1, the new pte value may become visible to the TLBs, and the
> > TLBs may allocate a new entry for it. Until step 4 completes, this entry
> > may remain active in the TLB, and may conflict with an existing entry.
> > 
> > If that entry covers the kernel text for steps 2-5, executing the
> > sequence may result in an unrecoverable TLB conflict abort, or some
> > other behaviour resulting from an amalgamated TLB, e.g. the I-cache
> > might fetch instructions from the wrong address such that steps 2-5
> > cannot be executed.
> > 
> > If the kernel doesn't explicitly access the address covered by that pte,
> > there may still be a problem. The TLB may perform an internal lookup
> > when performing a page table walk, and could then use an erroneous
> > result to continue the walk, resulting in a variety of potential issues
> > (e.g. reading from an MMIO peripheral register).
> > 
> > BBM avoids the conflict, but as that would mean kernel text and/or data
> > would be unmapped, you can't execute the code to finish the update.
> > 
> >> I find x86 does not have this limit. e.g. set_memory_r*.
> > 
> > I don't know much about x86; it's probably worth asking the x86 guys
> > about that. It may be that the x86 architecture requires that a conflict
> > or amalgamation is never visible to software, or it could be that
> > contemporary implementations happen to provide that property.
> > 
> > Thanks,
> > Mark.
> > 
> 
> Hi Mark,

Hi,

> Thank you for your reply, I find this code in /arch/arm64/mm/mmu.c
> 
> ...
> #ifdef CONFIG_DEBUG_RODATA
> void mark_rodata_ro(void)
> {
> 	create_mapping_late(__pa(_stext), (unsigned long)_stext,
> 				(unsigned long)_etext - (unsigned long)_stext,
> 				PAGE_KERNEL_EXEC | PTE_RDONLY);
> 
> }
> #endif
> ...
> 
> So does it also have this problem?

Currently, yes.

I've addressed the splitting/merging problem with my pagetable rework
series [1,2]. The RO region is initially mapped at the same granularity
as it will be modified with, so only the permissions bits will change
when mark_rodata_ro is called.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397095.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397114.html

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


#1308047

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-13 06:10 +0100
Message-ID<qQlR0-3iC-5@gated-at.bofh.it>
In reply to#1307281
On 2016/1/12 19:15, Mark Rutland wrote:

> On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
>> On 2016/1/11 21:31, Mark Rutland wrote:
>>
>>> Hi,
>>>
>>> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
>>>>
>>>> http://www.spinics.net/lists/arm-kernel/msg472090.html
>>>>
>>>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
>>>> and merging wiil produce confilct in the liner mapping area. Based on the
>>>> situation, Assume that set up page table in 4kb page table way in the liner
>>>> mapping area, Does the set_memroy_** will work without any conplict??
>>>
>>> I'm not sure I understand the question.
>>>
>>> I'm also not a fan of responding to off-list queries as information gets
>>> lost.
>>>
>>> Please ask your question on the mailing list. I am more than happy to
>>> respond there.
>>>
>>> Thanks,
>>> Mark.
>>>
>>
>> Hi Mark,
>>
>> In your patch it said "The presence of conflicting TLB entries may result in
>> a variety of behaviours detrimental to the system " and "but this(break-before-make
>> approach) cannot work for modifications to the swapper page tables that cover the
>> kernel text and data."
>>
>> I'm not quite understand this, why the direct mapping can't work?
> 
> The problem is that the TLB hardware can operate asynchronously to the
> rest of the CPU. At any point in time, for any reason, it can decide to
> destroy TLB entries, to allocate new ones, or to perform a walk based on
> the existing contents of the TLB.
> 
> When the TLB contains conflicting entries, TLB lookups may result in TLB
> conflict aborts, or may return an "amalgamation" of the conflicting
> entries (e.g. you could get an erroneous output address).
> 
> The direct mapping is in active use (and hence live in TLBs). Modifying
> it without break-before-make (BBM) risks the allocation of conflicting
> TLB entries. Modifying it with BBM risks unmapping the portion of the
> kernel performing the modification, resulting in an unrecoverable abort.
> 
>> flush tlb can't resolve it?
> 
> Flushing the TLB doesn't help because the page table update, TLB
> invalidate, and corresponding barrier(s) are separate operations. The
> TLB can allocate or destroy entries at any point during the sequence.
> 
> For example, without BBM a page table update would look something like:
> 
> 1)	str	<newpte>, [<*pte>]
> 2)	dsb	ish
> 3)	tlbi	vmalle1is
> 4)	dsb	ish
> 5)	isb
> 
> After step 1, the new pte value may become visible to the TLBs, and the
> TLBs may allocate a new entry for it. Until step 4 completes, this entry
> may remain active in the TLB, and may conflict with an existing entry.
> 
> If that entry covers the kernel text for steps 2-5, executing the
> sequence may result in an unrecoverable TLB conflict abort, or some
> other behaviour resulting from an amalgamated TLB, e.g. the I-cache
> might fetch instructions from the wrong address such that steps 2-5
> cannot be executed.
> 
> If the kernel doesn't explicitly access the address covered by that pte,
> there may still be a problem. The TLB may perform an internal lookup
> when performing a page table walk, and could then use an erroneous
> result to continue the walk, resulting in a variety of potential issues
> (e.g. reading from an MMIO peripheral register).
> 
> BBM avoids the conflict, but as that would mean kernel text and/or data
> would be unmapped, you can't execute the code to finish the update.
> 
>> I find x86 does not have this limit. e.g. set_memory_r*.
> 
> I don't know much about x86; it's probably worth asking the x86 guys
> about that. It may be that the x86 architecture requires that a conflict
> or amalgamation is never visible to software, or it could be that
> contemporary implementations happen to provide that property.
> 
> Thanks,
> Mark.
> 

Hi Mark,

If I do like this, does it have the problem too?

kmalloc a size
no access
flush tlb
call set_memory_ro to change the page table flag
flush tlb
start access

Thanks,
Xishi Qiu 

> .
> 

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


#1308082

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-13 07:40 +0100
Message-ID<qQng6-4bv-7@gated-at.bofh.it>
In reply to#1308047
On 2016/1/13 13:02, Xishi Qiu wrote:

> On 2016/1/12 19:15, Mark Rutland wrote:
> 
>> On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
>>> On 2016/1/11 21:31, Mark Rutland wrote:
>>>
>>>> Hi,
>>>>
>>>> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
>>>>>
>>>>> http://www.spinics.net/lists/arm-kernel/msg472090.html
>>>>>
>>>>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
>>>>> and merging wiil produce confilct in the liner mapping area. Based on the
>>>>> situation, Assume that set up page table in 4kb page table way in the liner
>>>>> mapping area, Does the set_memroy_** will work without any conplict??
>>>>
>>>> I'm not sure I understand the question.
>>>>
>>>> I'm also not a fan of responding to off-list queries as information gets
>>>> lost.
>>>>
>>>> Please ask your question on the mailing list. I am more than happy to
>>>> respond there.
>>>>
>>>> Thanks,
>>>> Mark.
>>>>
>>>
>>> Hi Mark,
>>>
>>> In your patch it said "The presence of conflicting TLB entries may result in
>>> a variety of behaviours detrimental to the system " and "but this(break-before-make
>>> approach) cannot work for modifications to the swapper page tables that cover the
>>> kernel text and data."
>>>
>>> I'm not quite understand this, why the direct mapping can't work?
>>
>> The problem is that the TLB hardware can operate asynchronously to the
>> rest of the CPU. At any point in time, for any reason, it can decide to
>> destroy TLB entries, to allocate new ones, or to perform a walk based on
>> the existing contents of the TLB.
>>
>> When the TLB contains conflicting entries, TLB lookups may result in TLB
>> conflict aborts, or may return an "amalgamation" of the conflicting
>> entries (e.g. you could get an erroneous output address).
>>
>> The direct mapping is in active use (and hence live in TLBs). Modifying
>> it without break-before-make (BBM) risks the allocation of conflicting
>> TLB entries. Modifying it with BBM risks unmapping the portion of the
>> kernel performing the modification, resulting in an unrecoverable abort.
>>
>>> flush tlb can't resolve it?
>>
>> Flushing the TLB doesn't help because the page table update, TLB
>> invalidate, and corresponding barrier(s) are separate operations. The
>> TLB can allocate or destroy entries at any point during the sequence.
>>
>> For example, without BBM a page table update would look something like:
>>
>> 1)	str	<newpte>, [<*pte>]
>> 2)	dsb	ish
>> 3)	tlbi	vmalle1is
>> 4)	dsb	ish
>> 5)	isb
>>
>> After step 1, the new pte value may become visible to the TLBs, and the
>> TLBs may allocate a new entry for it. Until step 4 completes, this entry
>> may remain active in the TLB, and may conflict with an existing entry.
>>
>> If that entry covers the kernel text for steps 2-5, executing the
>> sequence may result in an unrecoverable TLB conflict abort, or some
>> other behaviour resulting from an amalgamated TLB, e.g. the I-cache
>> might fetch instructions from the wrong address such that steps 2-5
>> cannot be executed.
>>
>> If the kernel doesn't explicitly access the address covered by that pte,
>> there may still be a problem. The TLB may perform an internal lookup
>> when performing a page table walk, and could then use an erroneous
>> result to continue the walk, resulting in a variety of potential issues
>> (e.g. reading from an MMIO peripheral register).
>>
>> BBM avoids the conflict, but as that would mean kernel text and/or data
>> would be unmapped, you can't execute the code to finish the update.
>>
>>> I find x86 does not have this limit. e.g. set_memory_r*.
>>
>> I don't know much about x86; it's probably worth asking the x86 guys
>> about that. It may be that the x86 architecture requires that a conflict
>> or amalgamation is never visible to software, or it could be that
>> contemporary implementations happen to provide that property.
>>
>> Thanks,
>> Mark.
>>
> 
> Hi Mark,
> 
> If I do like this, does it have the problem too?
> 
> kmalloc a size

exactly is alloc page, the size is aligned with page size.

> no access
> flush tlb
> call set_memory_ro to change the page table flag
> flush tlb
> start access
> 
> Thanks,
> Xishi Qiu 
> 
>> .
>>
> 
> 

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


#1308303

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-13 12:30 +0100
Message-ID<qQrMJ-7nW-11@gated-at.bofh.it>
In reply to#1308047
On Wed, Jan 13, 2016 at 01:02:31PM +0800, Xishi Qiu wrote:
> Hi Mark,
> 
> If I do like this, does it have the problem too?
> 
> kmalloc a size
> no access
> flush tlb
> call set_memory_ro to change the page table flag
> flush tlb
> start access

This is broken.

The kmalloc will give you memory form the linear mapping. Even if you
allocate a page, that page could have been mapped with a section at the
PMD/PUD/PGD level.

Other data could fall within that section (e.g. a kernel stack,
perhaps).

Additional TLB flushees do not help. There's still a race against the
asynchronous TLB logic. The TLB can allocate or destroy entries at any
tim. If there were no page table changes prior to the invalidate, the
TLB could re-allocate all existing entries immediately after the TLB
invalidate, leaving you in the same state as before.

Thanks,
Mark.

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


#1308273

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-13 11:40 +0100
Message-ID<qQr0n-6PJ-35@gated-at.bofh.it>
In reply to#1307281
On 2016/1/12 19:15, Mark Rutland wrote:

> On Tue, Jan 12, 2016 at 09:20:54AM +0800, Xishi Qiu wrote:
>> On 2016/1/11 21:31, Mark Rutland wrote:
>>
>>> Hi,
>>>
>>> On Mon, Jan 11, 2016 at 08:59:44PM +0800, zhong jiang wrote:
>>>>
>>>> http://www.spinics.net/lists/arm-kernel/msg472090.html
>>>>
>>>> Hi, Can I ask you a question? Say, This patch tells that the section spilting
>>>> and merging wiil produce confilct in the liner mapping area. Based on the
>>>> situation, Assume that set up page table in 4kb page table way in the liner
>>>> mapping area, Does the set_memroy_** will work without any conplict??
>>>
>>> I'm not sure I understand the question.
>>>
>>> I'm also not a fan of responding to off-list queries as information gets
>>> lost.
>>>
>>> Please ask your question on the mailing list. I am more than happy to
>>> respond there.
>>>
>>> Thanks,
>>> Mark.
>>>
>>
>> Hi Mark,
>>
>> In your patch it said "The presence of conflicting TLB entries may result in
>> a variety of behaviours detrimental to the system " and "but this(break-before-make
>> approach) cannot work for modifications to the swapper page tables that cover the
>> kernel text and data."
>>
>> I'm not quite understand this, why the direct mapping can't work?
> 
> The problem is that the TLB hardware can operate asynchronously to the
> rest of the CPU. At any point in time, for any reason, it can decide to
> destroy TLB entries, to allocate new ones, or to perform a walk based on
> the existing contents of the TLB.
> 
> When the TLB contains conflicting entries, TLB lookups may result in TLB
> conflict aborts, or may return an "amalgamation" of the conflicting
> entries (e.g. you could get an erroneous output address).
> 
> The direct mapping is in active use (and hence live in TLBs). Modifying
> it without break-before-make (BBM) risks the allocation of conflicting
> TLB entries. Modifying it with BBM risks unmapping the portion of the
> kernel performing the modification, resulting in an unrecoverable abort.
> 
>> flush tlb can't resolve it?
> 
> Flushing the TLB doesn't help because the page table update, TLB
> invalidate, and corresponding barrier(s) are separate operations. The
> TLB can allocate or destroy entries at any point during the sequence.
> 
> For example, without BBM a page table update would look something like:
> 
> 1)	str	<newpte>, [<*pte>]
> 2)	dsb	ish
> 3)	tlbi	vmalle1is
> 4)	dsb	ish
> 5)	isb
> 
> After step 1, the new pte value may become visible to the TLBs, and the
> TLBs may allocate a new entry for it. Until step 4 completes, this entry
> may remain active in the TLB, and may conflict with an existing entry.
> 
> If that entry covers the kernel text for steps 2-5, executing the
> sequence may result in an unrecoverable TLB conflict abort, or some
> other behaviour resulting from an amalgamated TLB, e.g. the I-cache
> might fetch instructions from the wrong address such that steps 2-5
> cannot be executed.
> 
> If the kernel doesn't explicitly access the address covered by that pte,
> there may still be a problem. The TLB may perform an internal lookup
> when performing a page table walk, and could then use an erroneous
> result to continue the walk, resulting in a variety of potential issues
> (e.g. reading from an MMIO peripheral register).
> 
> BBM avoids the conflict, but as that would mean kernel text and/or data
> would be unmapped, you can't execute the code to finish the update.
> 
>> I find x86 does not have this limit. e.g. set_memory_r*.
> 
> I don't know much about x86; it's probably worth asking the x86 guys
> about that. It may be that the x86 architecture requires that a conflict
> or amalgamation is never visible to software, or it could be that
> contemporary implementations happen to provide that property.
> 
> Thanks,
> Mark.
> 

Hi Mark,

If I create swapper page tables by 4kb, not large page, then I use
set_memory_ro() to change the pate table flag, does it have the problem
too?

Thanks,
Xishi Qiu
 

> .
> 

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


#1308297

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-13 12:20 +0100
Message-ID<qQrD4-7jL-19@gated-at.bofh.it>
In reply to#1308273
On Wed, Jan 13, 2016 at 06:30:06PM +0800, Xishi Qiu wrote:
> Hi Mark,
> 
> If I create swapper page tables by 4kb, not large page, then I use
> set_memory_ro() to change the pate table flag, does it have the problem
> too?

The splitting/merging problem would not apply.

However, you're going to waste a reasonable amount of memory by not
using section mappings in the swapper, and we gain additional complexity
in the page table setup code (which is shared with others things that
want section mappings).

What are you exactly actually trying to achieve?

What memory do you want to mark RO, and why?

From a previous discussion [1], we figured out alternative approaches
for common cases. Do none of those work for your case?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397320.html

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


#1309239

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-14 13:40 +0100
Message-ID<qQPm3-71u-23@gated-at.bofh.it>
In reply to#1308297
On 2016/1/13 19:18, Mark Rutland wrote:

> On Wed, Jan 13, 2016 at 06:30:06PM +0800, Xishi Qiu wrote:
>> Hi Mark,
>>
>> If I create swapper page tables by 4kb, not large page, then I use
>> set_memory_ro() to change the pate table flag, does it have the problem
>> too?
> 
> The splitting/merging problem would not apply.
> 
> However, you're going to waste a reasonable amount of memory by not
> using section mappings in the swapper, and we gain additional complexity
> in the page table setup code (which is shared with others things that
> want section mappings).
> 
> What are you exactly actually trying to achieve?
> 

If module allocates some pages and save data on them, and the data will
not be changed during the module running. So we want to use set_memory_ro()
to increase the security. If the data is changed, we can catch someone.

> What memory do you want to mark RO, and why?
> 

The key data, and it will not be changed during the running time.

>>From a previous discussion [1], we figured out alternative approaches
> for common cases. Do none of those work for your case?
> 

I have not read the patchset carefully, could you tell me the general meaning
of the approaches?

Thanks,
Xishi Qiu

> Thanks,
> Mark.
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397320.html
> 
> .
> 

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


#1309261

FromXishi Qiu <qiuxishi@huawei.com>
Date2016-01-14 14:10 +0100
Message-ID<qQPP4-7uv-7@gated-at.bofh.it>
In reply to#1309239
On 2016/1/14 20:35, Xishi Qiu wrote:

> On 2016/1/13 19:18, Mark Rutland wrote:
> 
>> On Wed, Jan 13, 2016 at 06:30:06PM +0800, Xishi Qiu wrote:
>>> Hi Mark,
>>>
>>> If I create swapper page tables by 4kb, not large page, then I use
>>> set_memory_ro() to change the pate table flag, does it have the problem
>>> too?
>>
>> The splitting/merging problem would not apply.
>>
>> However, you're going to waste a reasonable amount of memory by not
>> using section mappings in the swapper, and we gain additional complexity
>> in the page table setup code (which is shared with others things that
>> want section mappings).
>>
>> What are you exactly actually trying to achieve?
>>
> 
> If module allocates some pages and save data on them, and the data will
> not be changed during the module running. So we want to use set_memory_ro()
> to increase the security. If the data is changed, we can catch someone.
> 
>> What memory do you want to mark RO, and why?
>>
> 
> The key data, and it will not be changed during the running time.
> 
>> >From a previous discussion [1], we figured out alternative approaches
>> for common cases. Do none of those work for your case?
>>
> 
> I have not read the patchset carefully, could you tell me the general meaning
> of the approaches?
> 

Hi Mark,

Is the two approaches like following?
1. use create_mapping to map the data in read only, then use fixmap to create a
temp page table, and change the data when necessary.
2. use vmalloc, then we can use set_memory_ro to change the page table prot.

Thanks,
Xishi Qiu

> Thanks,
> Xishi Qiu
> 
>> Thanks,
>> Mark.
>>
>> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397320.html
>>
>> .
>>
> 
> 

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


#1309289

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-14 14:50 +0100
Message-ID<qQQrM-7NG-23@gated-at.bofh.it>
In reply to#1309261
On Thu, Jan 14, 2016 at 09:06:08PM +0800, Xishi Qiu wrote:
> On 2016/1/14 20:35, Xishi Qiu wrote:
> 
> > On 2016/1/13 19:18, Mark Rutland wrote:
> > 
> >> On Wed, Jan 13, 2016 at 06:30:06PM +0800, Xishi Qiu wrote:
> >>> Hi Mark,
> >>>
> >>> If I create swapper page tables by 4kb, not large page, then I use
> >>> set_memory_ro() to change the pate table flag, does it have the problem
> >>> too?
> >>
> >> The splitting/merging problem would not apply.
> >>
> >> However, you're going to waste a reasonable amount of memory by not
> >> using section mappings in the swapper, and we gain additional complexity
> >> in the page table setup code (which is shared with others things that
> >> want section mappings).
> >>
> >> What are you exactly actually trying to achieve?
> >>
> > 
> > If module allocates some pages and save data on them, and the data will
> > not be changed during the module running. So we want to use set_memory_ro()
> > to increase the security. If the data is changed, we can catch someone.
> > 
> >> What memory do you want to mark RO, and why?
> >>
> > 
> > The key data, and it will not be changed during the running time.
> > 
> >> >From a previous discussion [1], we figured out alternative approaches
> >> for common cases. Do none of those work for your case?
> >>
> > 
> > I have not read the patchset carefully, could you tell me the general meaning
> > of the approaches?
> > 
> 
> Hi Mark,
> 
> Is the two approaches like following?
> 1. use create_mapping to map the data in read only, then use fixmap to create a
> temp page table, and change the data when necessary.

In your code you'd have to statically place the data in .rodata somehow
(e.g. [2]). Your code would not call create_mapping. The usual init code
would take care of that.

Note that this can only work for a fixed amount of data, whereas it
sounds like you are doing dynamic allocation.

> 2. use vmalloc, then we can use set_memory_ro to change the page table prot.

Something like this should be workable, yes. See [3,4].

> >> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/397320.html

[2] https://lkml.org/lkml/2015/11/24/724
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/399015.html
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/399252.html

Thanks,
Mark.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web