| Message ID | b213b4d4dcec41508ccad9e42edf6d234495a73b.1424091289.git.jan.kiszka@siemens.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: > From: Ian Campbell <ijc@hellion.org.uk> > > In this case the secure code lives in RAM, and hence needs to be reserved, but > it has been relocated, so the reservation of __secure_start does not apply. > > Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a > region. > > This will be used in a subsequent patch for Jetson-TK1 Using a memreserve and allowing the OS to map the memory but not poke it can be problematic due to the potential of mismatched attributes between the monitor and the OS. If you're able to carve out the "secure" memory from the memory node(s), then you should be safe from that. Thanks, Mark. > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/arm/cpu/armv7/virt-dt.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c > index ad19e4c..eb95031 100644 > --- a/arch/arm/cpu/armv7/virt-dt.c > +++ b/arch/arm/cpu/armv7/virt-dt.c > @@ -96,6 +96,11 @@ int armv7_update_dt(void *fdt) > /* secure code lives in RAM, keep it alive */ > fdt_add_mem_rsv(fdt, (unsigned long)__secure_start, > __secure_end - __secure_start); > +#elif defined(CONFIG_ARMV7_SECURE_RESERVE_SIZE) > + /* secure code has been relocated into RAM carveout, keep it alive */ > + fdt_add_mem_rsv(fdt, > + CONFIG_ARMV7_SECURE_BASE, > + CONFIG_ARMV7_SECURE_RESERVE_SIZE); > #endif > > return fdt_psci(fdt); > -- > 2.1.4 > >
On 2015-02-16 14:42, Mark Rutland wrote: > On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >> From: Ian Campbell <ijc@hellion.org.uk> >> >> In this case the secure code lives in RAM, and hence needs to be reserved, but >> it has been relocated, so the reservation of __secure_start does not apply. >> >> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >> region. >> >> This will be used in a subsequent patch for Jetson-TK1 > > Using a memreserve and allowing the OS to map the memory but not poke it > can be problematic due to the potential of mismatched attributes between > the monitor and the OS. OK, here my knowledge is not yet sufficient to process this remark. What kind of problems can arise from what kind of attribute mismatch? And why should the OS be able to cause problems for the monitor? > > If you're able to carve out the "secure" memory from the memory node(s), > then you should be safe from that. Do you have a pointer to an example how to do it instead? Jan > > Thanks, > Mark. > >> >> Signed-off-by: Ian Campbell <ijc@hellion.org.uk> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/arm/cpu/armv7/virt-dt.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c >> index ad19e4c..eb95031 100644 >> --- a/arch/arm/cpu/armv7/virt-dt.c >> +++ b/arch/arm/cpu/armv7/virt-dt.c >> @@ -96,6 +96,11 @@ int armv7_update_dt(void *fdt) >> /* secure code lives in RAM, keep it alive */ >> fdt_add_mem_rsv(fdt, (unsigned long)__secure_start, >> __secure_end - __secure_start); >> +#elif defined(CONFIG_ARMV7_SECURE_RESERVE_SIZE) >> + /* secure code has been relocated into RAM carveout, keep it alive */ >> + fdt_add_mem_rsv(fdt, >> + CONFIG_ARMV7_SECURE_BASE, >> + CONFIG_ARMV7_SECURE_RESERVE_SIZE); >> #endif >> >> return fdt_psci(fdt); >> -- >> 2.1.4 >> >>
On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: > On 2015-02-16 14:42, Mark Rutland wrote: > > On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: > >> From: Ian Campbell <ijc@hellion.org.uk> > >> > >> In this case the secure code lives in RAM, and hence needs to be reserved, but > >> it has been relocated, so the reservation of __secure_start does not apply. > >> > >> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a > >> region. > >> > >> This will be used in a subsequent patch for Jetson-TK1 > > > > Using a memreserve and allowing the OS to map the memory but not poke it > > can be problematic due to the potential of mismatched attributes between > > the monitor and the OS. > > OK, here my knowledge is not yet sufficient to process this remark. What > kind of problems can arise from what kind of attribute mismatch? And why > should the OS be able to cause problems for the monitor? For example, consider the case of the region being mapped cacheable by the OS but not by the monitor. The monitor communicates between cores expecting to never hit in a cache (because it uses a non-cacheable mapping), but the mapping used by the OS can cause the region to be allocated into caches at any point in time even if it never accesses the region explicitly. The CPU _may_ hit in a cache even if making a non-cacheable access (this is called an "unexepcted data cache hit"), so the cache allocations caused by the OS can mask data other CPUs wrote straight to memory. Other than that case, I believe the rules given in the ARM ARM for mismatched memory attributes may apply for similar reasons. Thus allowing the OS to map this memory can cause a loss of coherency on the monitor side, if the OS and monitor map the region with different attributes. This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the system you're dealing with. I don't immediately know whether that is the case, however. Never telling the OS about the memory in the first place avoids the possibility in all cases. > > If you're able to carve out the "secure" memory from the memory node(s), > > then you should be safe from that. > > Do you have a pointer to an example how to do it instead? Unfortunately not; I don't know whether there's an existing primitive for doing that. In general you might need to split a memory region in two to carve out the portion in the middle. Thanks, Mark.
On 2015-02-16 15:25, Mark Rutland wrote: > On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: >> On 2015-02-16 14:42, Mark Rutland wrote: >>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >>>> From: Ian Campbell <ijc@hellion.org.uk> >>>> >>>> In this case the secure code lives in RAM, and hence needs to be reserved, but >>>> it has been relocated, so the reservation of __secure_start does not apply. >>>> >>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >>>> region. >>>> >>>> This will be used in a subsequent patch for Jetson-TK1 >>> >>> Using a memreserve and allowing the OS to map the memory but not poke it >>> can be problematic due to the potential of mismatched attributes between >>> the monitor and the OS. >> >> OK, here my knowledge is not yet sufficient to process this remark. What >> kind of problems can arise from what kind of attribute mismatch? And why >> should the OS be able to cause problems for the monitor? > > For example, consider the case of the region being mapped cacheable by > the OS but not by the monitor. The monitor communicates between cores > expecting to never hit in a cache (because it uses a non-cacheable > mapping), but the mapping used by the OS can cause the region to be > allocated into caches at any point in time even if it never accesses the > region explicitly. > > The CPU _may_ hit in a cache even if making a non-cacheable access (this > is called an "unexepcted data cache hit"), so the cache allocations > caused by the OS can mask data other CPUs wrote straight to memory. > > Other than that case, I believe the rules given in the ARM ARM for > mismatched memory attributes may apply for similar reasons. Thus > allowing the OS to map this memory can cause a loss of coherency on the > monitor side, if the OS and monitor map the region with different > attributes. > > This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the > system you're dealing with. I don't immediately know whether that is the > case, however. Never telling the OS about the memory in the first place > avoids the possibility in all cases. But from a security point of view, it must not matter if the OS maps the memory or not - the monitor must be robust against that, no? If the architecture cannot provide such guarantees, it has to be worked around in software in the monitor (I hope you can do so...). Jan > >>> If you're able to carve out the "secure" memory from the memory node(s), >>> then you should be safe from that. >> >> Do you have a pointer to an example how to do it instead? > > Unfortunately not; I don't know whether there's an existing primitive > for doing that. In general you might need to split a memory region in > two to carve out the portion in the middle. > > Thanks, > Mark. >
On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: > On 2015-02-16 15:25, Mark Rutland wrote: > > On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: > >> On 2015-02-16 14:42, Mark Rutland wrote: > >>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: > >>>> From: Ian Campbell <ijc@hellion.org.uk> > >>>> > >>>> In this case the secure code lives in RAM, and hence needs to be reserved, but > >>>> it has been relocated, so the reservation of __secure_start does not apply. > >>>> > >>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a > >>>> region. > >>>> > >>>> This will be used in a subsequent patch for Jetson-TK1 > >>> > >>> Using a memreserve and allowing the OS to map the memory but not poke it > >>> can be problematic due to the potential of mismatched attributes between > >>> the monitor and the OS. > >> > >> OK, here my knowledge is not yet sufficient to process this remark. What > >> kind of problems can arise from what kind of attribute mismatch? And why > >> should the OS be able to cause problems for the monitor? > > > > For example, consider the case of the region being mapped cacheable by > > the OS but not by the monitor. The monitor communicates between cores > > expecting to never hit in a cache (because it uses a non-cacheable > > mapping), but the mapping used by the OS can cause the region to be > > allocated into caches at any point in time even if it never accesses the > > region explicitly. > > > > The CPU _may_ hit in a cache even if making a non-cacheable access (this > > is called an "unexepcted data cache hit"), so the cache allocations > > caused by the OS can mask data other CPUs wrote straight to memory. > > > > Other than that case, I believe the rules given in the ARM ARM for > > mismatched memory attributes may apply for similar reasons. Thus > > allowing the OS to map this memory can cause a loss of coherency on the > > monitor side, if the OS and monitor map the region with different > > attributes. > > > > This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the > > system you're dealing with. I don't immediately know whether that is the > > case, however. Never telling the OS about the memory in the first place > > avoids the possibility in all cases. > > But from a security point of view, it must not matter if the OS maps the > memory or not - the monitor must be robust against that, no? If the > architecture cannot provide such guarantees, it has to be worked around > in software in the monitor (I hope you can do so...). Well, yes and no. In this case it sounds like due to the security controller you should never encounter the mismatched attributes issue in the first place, though you may encounter issues w.r.t. speculative accesses triggering violations arbitrarily. Not telling the OS about the secure memory means that said violations shouldn't occur in normal operation; only when the non-secure OS is trying to do something bad. If the OS has access to the memory, then you're already trusting it to not write to there or you can't trust that memory at all (and hence cannot use it). Given that means you must already assume that the OS is cooperative, it's simpler to not tell it about the memory than to add cache maintenance around every memory access within the monitor. You can never make things secure in this case, but you can at least offer the abstraction provided by PSCI. So as far as I can see in either case it's better to not tell the OS about the memory you wish to use from the monitor. If you have no HW protection and can't trust the OS then you've already lost, and if you do have HW protection you don't want it to trigger continuously/spuriously as a result of speculation. Thanks, Mark.
On 2015-02-16 15:56, Mark Rutland wrote: > On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: >> On 2015-02-16 15:25, Mark Rutland wrote: >>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: >>>> On 2015-02-16 14:42, Mark Rutland wrote: >>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >>>>>> From: Ian Campbell <ijc@hellion.org.uk> >>>>>> >>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but >>>>>> it has been relocated, so the reservation of __secure_start does not apply. >>>>>> >>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >>>>>> region. >>>>>> >>>>>> This will be used in a subsequent patch for Jetson-TK1 >>>>> >>>>> Using a memreserve and allowing the OS to map the memory but not poke it >>>>> can be problematic due to the potential of mismatched attributes between >>>>> the monitor and the OS. >>>> >>>> OK, here my knowledge is not yet sufficient to process this remark. What >>>> kind of problems can arise from what kind of attribute mismatch? And why >>>> should the OS be able to cause problems for the monitor? >>> >>> For example, consider the case of the region being mapped cacheable by >>> the OS but not by the monitor. The monitor communicates between cores >>> expecting to never hit in a cache (because it uses a non-cacheable >>> mapping), but the mapping used by the OS can cause the region to be >>> allocated into caches at any point in time even if it never accesses the >>> region explicitly. >>> >>> The CPU _may_ hit in a cache even if making a non-cacheable access (this >>> is called an "unexepcted data cache hit"), so the cache allocations >>> caused by the OS can mask data other CPUs wrote straight to memory. >>> >>> Other than that case, I believe the rules given in the ARM ARM for >>> mismatched memory attributes may apply for similar reasons. Thus >>> allowing the OS to map this memory can cause a loss of coherency on the >>> monitor side, if the OS and monitor map the region with different >>> attributes. >>> >>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the >>> system you're dealing with. I don't immediately know whether that is the >>> case, however. Never telling the OS about the memory in the first place >>> avoids the possibility in all cases. >> >> But from a security point of view, it must not matter if the OS maps the >> memory or not - the monitor must be robust against that, no? If the >> architecture cannot provide such guarantees, it has to be worked around >> in software in the monitor (I hope you can do so...). > > Well, yes and no. > > In this case it sounds like due to the security controller you should > never encounter the mismatched attributes issue in the first place, > though you may encounter issues w.r.t. speculative accesses triggering > violations arbitrarily. Not telling the OS about the secure memory means > that said violations shouldn't occur in normal operation; only when the > non-secure OS is trying to do something bad. > > If the OS has access to the memory, then you're already trusting it to > not write to there or you can't trust that memory at all (and hence > cannot use it). Given that means you must already assume that the OS is > cooperative, it's simpler to not tell it about the memory than to add > cache maintenance around every memory access within the monitor. You can > never make things secure in this case, but you can at least offer the > abstraction provided by PSCI. > > So as far as I can see in either case it's better to not tell the OS > about the memory you wish to use from the monitor. If you have no HW > protection and can't trust the OS then you've already lost, and if you > do have HW protection you don't want it to trigger > continuously/spuriously as a result of speculation. OK, that makes sense again. Now I just need to figure out how to split/adjust the memory node instead of adding a reservation region. Jan
On 2015-02-16 16:38, Jan Kiszka wrote: > On 2015-02-16 15:56, Mark Rutland wrote: >> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: >>> On 2015-02-16 15:25, Mark Rutland wrote: >>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: >>>>> On 2015-02-16 14:42, Mark Rutland wrote: >>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >>>>>>> From: Ian Campbell <ijc@hellion.org.uk> >>>>>>> >>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but >>>>>>> it has been relocated, so the reservation of __secure_start does not apply. >>>>>>> >>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >>>>>>> region. >>>>>>> >>>>>>> This will be used in a subsequent patch for Jetson-TK1 >>>>>> >>>>>> Using a memreserve and allowing the OS to map the memory but not poke it >>>>>> can be problematic due to the potential of mismatched attributes between >>>>>> the monitor and the OS. >>>>> >>>>> OK, here my knowledge is not yet sufficient to process this remark. What >>>>> kind of problems can arise from what kind of attribute mismatch? And why >>>>> should the OS be able to cause problems for the monitor? >>>> >>>> For example, consider the case of the region being mapped cacheable by >>>> the OS but not by the monitor. The monitor communicates between cores >>>> expecting to never hit in a cache (because it uses a non-cacheable >>>> mapping), but the mapping used by the OS can cause the region to be >>>> allocated into caches at any point in time even if it never accesses the >>>> region explicitly. >>>> >>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this >>>> is called an "unexepcted data cache hit"), so the cache allocations >>>> caused by the OS can mask data other CPUs wrote straight to memory. >>>> >>>> Other than that case, I believe the rules given in the ARM ARM for >>>> mismatched memory attributes may apply for similar reasons. Thus >>>> allowing the OS to map this memory can cause a loss of coherency on the >>>> monitor side, if the OS and monitor map the region with different >>>> attributes. >>>> >>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the >>>> system you're dealing with. I don't immediately know whether that is the >>>> case, however. Never telling the OS about the memory in the first place >>>> avoids the possibility in all cases. >>> >>> But from a security point of view, it must not matter if the OS maps the >>> memory or not - the monitor must be robust against that, no? If the >>> architecture cannot provide such guarantees, it has to be worked around >>> in software in the monitor (I hope you can do so...). >> >> Well, yes and no. >> >> In this case it sounds like due to the security controller you should >> never encounter the mismatched attributes issue in the first place, >> though you may encounter issues w.r.t. speculative accesses triggering >> violations arbitrarily. Not telling the OS about the secure memory means >> that said violations shouldn't occur in normal operation; only when the >> non-secure OS is trying to do something bad. >> >> If the OS has access to the memory, then you're already trusting it to >> not write to there or you can't trust that memory at all (and hence >> cannot use it). Given that means you must already assume that the OS is >> cooperative, it's simpler to not tell it about the memory than to add >> cache maintenance around every memory access within the monitor. You can >> never make things secure in this case, but you can at least offer the >> abstraction provided by PSCI. >> >> So as far as I can see in either case it's better to not tell the OS >> about the memory you wish to use from the monitor. If you have no HW >> protection and can't trust the OS then you've already lost, and if you >> do have HW protection you don't want it to trigger >> continuously/spuriously as a result of speculation. > > OK, that makes sense again. > > Now I just need to figure out how to split/adjust the memory node > instead of adding a reservation region. This is getting invasive: If I add carveouts via adjusting memory banks, I need to account for the case that an existing bank is split into two halves, creating additional banks this way. But then current fdt_fixup_memory_banks will no longer work due to its limitation to the number of physical banks. I could always add one spare bank to that service, ok, but then the next use case for carveouts will hit the wall again. So I better double that limit, or so. Also, are there any architectural or OS-implementation related restrictions on the alignment of bank start addresses and sizes? Just to make sure we don't stumble over some side effects of punching holes into that device tree node. Jan
On Tue, Feb 17, 2015 at 08:09:57AM +0000, Jan Kiszka wrote: > On 2015-02-16 16:38, Jan Kiszka wrote: > > On 2015-02-16 15:56, Mark Rutland wrote: > >> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: > >>> On 2015-02-16 15:25, Mark Rutland wrote: > >>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: > >>>>> On 2015-02-16 14:42, Mark Rutland wrote: > >>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: > >>>>>>> From: Ian Campbell <ijc@hellion.org.uk> > >>>>>>> > >>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but > >>>>>>> it has been relocated, so the reservation of __secure_start does not apply. > >>>>>>> > >>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a > >>>>>>> region. > >>>>>>> > >>>>>>> This will be used in a subsequent patch for Jetson-TK1 > >>>>>> > >>>>>> Using a memreserve and allowing the OS to map the memory but not poke it > >>>>>> can be problematic due to the potential of mismatched attributes between > >>>>>> the monitor and the OS. > >>>>> > >>>>> OK, here my knowledge is not yet sufficient to process this remark. What > >>>>> kind of problems can arise from what kind of attribute mismatch? And why > >>>>> should the OS be able to cause problems for the monitor? > >>>> > >>>> For example, consider the case of the region being mapped cacheable by > >>>> the OS but not by the monitor. The monitor communicates between cores > >>>> expecting to never hit in a cache (because it uses a non-cacheable > >>>> mapping), but the mapping used by the OS can cause the region to be > >>>> allocated into caches at any point in time even if it never accesses the > >>>> region explicitly. > >>>> > >>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this > >>>> is called an "unexepcted data cache hit"), so the cache allocations > >>>> caused by the OS can mask data other CPUs wrote straight to memory. > >>>> > >>>> Other than that case, I believe the rules given in the ARM ARM for > >>>> mismatched memory attributes may apply for similar reasons. Thus > >>>> allowing the OS to map this memory can cause a loss of coherency on the > >>>> monitor side, if the OS and monitor map the region with different > >>>> attributes. > >>>> > >>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the > >>>> system you're dealing with. I don't immediately know whether that is the > >>>> case, however. Never telling the OS about the memory in the first place > >>>> avoids the possibility in all cases. > >>> > >>> But from a security point of view, it must not matter if the OS maps the > >>> memory or not - the monitor must be robust against that, no? If the > >>> architecture cannot provide such guarantees, it has to be worked around > >>> in software in the monitor (I hope you can do so...). > >> > >> Well, yes and no. > >> > >> In this case it sounds like due to the security controller you should > >> never encounter the mismatched attributes issue in the first place, > >> though you may encounter issues w.r.t. speculative accesses triggering > >> violations arbitrarily. Not telling the OS about the secure memory means > >> that said violations shouldn't occur in normal operation; only when the > >> non-secure OS is trying to do something bad. > >> > >> If the OS has access to the memory, then you're already trusting it to > >> not write to there or you can't trust that memory at all (and hence > >> cannot use it). Given that means you must already assume that the OS is > >> cooperative, it's simpler to not tell it about the memory than to add > >> cache maintenance around every memory access within the monitor. You can > >> never make things secure in this case, but you can at least offer the > >> abstraction provided by PSCI. > >> > >> So as far as I can see in either case it's better to not tell the OS > >> about the memory you wish to use from the monitor. If you have no HW > >> protection and can't trust the OS then you've already lost, and if you > >> do have HW protection you don't want it to trigger > >> continuously/spuriously as a result of speculation. > > > > OK, that makes sense again. > > > > Now I just need to figure out how to split/adjust the memory node > > instead of adding a reservation region. > > This is getting invasive: > > If I add carveouts via adjusting memory banks, I need to account for the > case that an existing bank is split into two halves, creating additional > banks this way. But then current fdt_fixup_memory_banks will no longer > work due to its limitation to the number of physical banks. I could > always add one spare bank to that service, ok, but then the next use > case for carveouts will hit the wall again. So I better double that > limit, or so. Yeah, not fun. If the code is position-independent then you might be able to simply carve out a sufficient proportion from the start of the first entry or the end of the last one, which would avoid splitting. If either of said regions are too small for the monitor code then it's questionable as to whether the OS can make use of it. > Also, are there any architectural or OS-implementation related > restrictions on the alignment of bank start addresses and sizes? Just to > make sure we don't stumble over some side effects of punching holes into > that device tree node. I would guess that we need to at least pad the carevout to page-aligned to prevent any particular OS from mapping a page for the sake of a few bytes left unused by the monitor. From a quick look at the Linux arm_add_memory and memblock code it looks like Linux won't map partial pages, but I don't know what Xen and others do, and given we know that we want to keep the relevant pages exclusive to the monitor anyway padding to age boundaries seems like a sensible thing to do. My one concern would be early mappings; I believe that the initial page tables use (2MiB) section/block mappings to map the kernel and some initial memory (including the DTB) before the memory nodes are parsed, so the carevout would need to be placed away from where the kernel and DTB were loaded in order to prevent those early mappings from covering it. I'm unfortunately not sure on the full details there. Thanks, Mark.
On 2015-02-17 11:46, Mark Rutland wrote: > On Tue, Feb 17, 2015 at 08:09:57AM +0000, Jan Kiszka wrote: >> On 2015-02-16 16:38, Jan Kiszka wrote: >>> On 2015-02-16 15:56, Mark Rutland wrote: >>>> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: >>>>> On 2015-02-16 15:25, Mark Rutland wrote: >>>>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: >>>>>>> On 2015-02-16 14:42, Mark Rutland wrote: >>>>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >>>>>>>>> From: Ian Campbell <ijc@hellion.org.uk> >>>>>>>>> >>>>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but >>>>>>>>> it has been relocated, so the reservation of __secure_start does not apply. >>>>>>>>> >>>>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >>>>>>>>> region. >>>>>>>>> >>>>>>>>> This will be used in a subsequent patch for Jetson-TK1 >>>>>>>> >>>>>>>> Using a memreserve and allowing the OS to map the memory but not poke it >>>>>>>> can be problematic due to the potential of mismatched attributes between >>>>>>>> the monitor and the OS. >>>>>>> >>>>>>> OK, here my knowledge is not yet sufficient to process this remark. What >>>>>>> kind of problems can arise from what kind of attribute mismatch? And why >>>>>>> should the OS be able to cause problems for the monitor? >>>>>> >>>>>> For example, consider the case of the region being mapped cacheable by >>>>>> the OS but not by the monitor. The monitor communicates between cores >>>>>> expecting to never hit in a cache (because it uses a non-cacheable >>>>>> mapping), but the mapping used by the OS can cause the region to be >>>>>> allocated into caches at any point in time even if it never accesses the >>>>>> region explicitly. >>>>>> >>>>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this >>>>>> is called an "unexepcted data cache hit"), so the cache allocations >>>>>> caused by the OS can mask data other CPUs wrote straight to memory. >>>>>> >>>>>> Other than that case, I believe the rules given in the ARM ARM for >>>>>> mismatched memory attributes may apply for similar reasons. Thus >>>>>> allowing the OS to map this memory can cause a loss of coherency on the >>>>>> monitor side, if the OS and monitor map the region with different >>>>>> attributes. >>>>>> >>>>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the >>>>>> system you're dealing with. I don't immediately know whether that is the >>>>>> case, however. Never telling the OS about the memory in the first place >>>>>> avoids the possibility in all cases. >>>>> >>>>> But from a security point of view, it must not matter if the OS maps the >>>>> memory or not - the monitor must be robust against that, no? If the >>>>> architecture cannot provide such guarantees, it has to be worked around >>>>> in software in the monitor (I hope you can do so...). >>>> >>>> Well, yes and no. >>>> >>>> In this case it sounds like due to the security controller you should >>>> never encounter the mismatched attributes issue in the first place, >>>> though you may encounter issues w.r.t. speculative accesses triggering >>>> violations arbitrarily. Not telling the OS about the secure memory means >>>> that said violations shouldn't occur in normal operation; only when the >>>> non-secure OS is trying to do something bad. >>>> >>>> If the OS has access to the memory, then you're already trusting it to >>>> not write to there or you can't trust that memory at all (and hence >>>> cannot use it). Given that means you must already assume that the OS is >>>> cooperative, it's simpler to not tell it about the memory than to add >>>> cache maintenance around every memory access within the monitor. You can >>>> never make things secure in this case, but you can at least offer the >>>> abstraction provided by PSCI. >>>> >>>> So as far as I can see in either case it's better to not tell the OS >>>> about the memory you wish to use from the monitor. If you have no HW >>>> protection and can't trust the OS then you've already lost, and if you >>>> do have HW protection you don't want it to trigger >>>> continuously/spuriously as a result of speculation. >>> >>> OK, that makes sense again. >>> >>> Now I just need to figure out how to split/adjust the memory node >>> instead of adding a reservation region. >> >> This is getting invasive: >> >> If I add carveouts via adjusting memory banks, I need to account for the >> case that an existing bank is split into two halves, creating additional >> banks this way. But then current fdt_fixup_memory_banks will no longer >> work due to its limitation to the number of physical banks. I could >> always add one spare bank to that service, ok, but then the next use >> case for carveouts will hit the wall again. So I better double that >> limit, or so. > > Yeah, not fun. > > If the code is position-independent then you might be able to simply > carve out a sufficient proportion from the start of the first entry or > the end of the last one, which would avoid splitting. If either of said > regions are too small for the monitor code then it's questionable as to > whether the OS can make use of it. The code /seems/ to be position-independent, but locations are so far hard-coded in those places that prepare it and move it around. Maybe we can decide about the location at runtime, maybe we can simply demand it to be at the end or the beginning of some bank. > >> Also, are there any architectural or OS-implementation related >> restrictions on the alignment of bank start addresses and sizes? Just to >> make sure we don't stumble over some side effects of punching holes into >> that device tree node. > > I would guess that we need to at least pad the carevout to page-aligned > to prevent any particular OS from mapping a page for the sake of a few > bytes left unused by the monitor. > > From a quick look at the Linux arm_add_memory and memblock code it looks > like Linux won't map partial pages, but I don't know what Xen and others > do, and given we know that we want to keep the relevant pages exclusive > to the monitor anyway padding to age boundaries seems like a sensible > thing to do. > > My one concern would be early mappings; I believe that the initial page > tables use (2MiB) section/block mappings to map the kernel and some > initial memory (including the DTB) before the memory nodes are parsed, > so the carevout would need to be placed away from where the kernel and > DTB were loaded in order to prevent those early mappings from covering > it. I'm unfortunately not sure on the full details there. That makes be wonder again if we are trying to solve real issues: What is the OS supposed to do with a memory reserve map, what does it have to avoid doing with it? Is the semantic really so weak that we cannot use it here? Jan
[...] > >> This is getting invasive: > >> > >> If I add carveouts via adjusting memory banks, I need to account for the > >> case that an existing bank is split into two halves, creating additional > >> banks this way. But then current fdt_fixup_memory_banks will no longer > >> work due to its limitation to the number of physical banks. I could > >> always add one spare bank to that service, ok, but then the next use > >> case for carveouts will hit the wall again. So I better double that > >> limit, or so. > > > > Yeah, not fun. > > > > If the code is position-independent then you might be able to simply > > carve out a sufficient proportion from the start of the first entry or > > the end of the last one, which would avoid splitting. If either of said > > regions are too small for the monitor code then it's questionable as to > > whether the OS can make use of it. > > The code /seems/ to be position-independent, but locations are so far > hard-coded in those places that prepare it and move it around. Maybe we > can decide about the location at runtime, maybe we can simply demand it > to be at the end or the beginning of some bank. If it's possible to do so, it would seem like the nicest option to me. > >> Also, are there any architectural or OS-implementation related > >> restrictions on the alignment of bank start addresses and sizes? Just to > >> make sure we don't stumble over some side effects of punching holes into > >> that device tree node. > > > > I would guess that we need to at least pad the carevout to page-aligned > > to prevent any particular OS from mapping a page for the sake of a few > > bytes left unused by the monitor. > > > > From a quick look at the Linux arm_add_memory and memblock code it looks > > like Linux won't map partial pages, but I don't know what Xen and others > > do, and given we know that we want to keep the relevant pages exclusive > > to the monitor anyway padding to age boundaries seems like a sensible > > thing to do. > > > > My one concern would be early mappings; I believe that the initial page > > tables use (2MiB) section/block mappings to map the kernel and some > > initial memory (including the DTB) before the memory nodes are parsed, > > so the carevout would need to be placed away from where the kernel and > > DTB were loaded in order to prevent those early mappings from covering > > it. I'm unfortunately not sure on the full details there. > > That makes be wonder again if we are trying to solve real issues: What > is the OS supposed to do with a memory reserve map, what does it have to > avoid doing with it? Per ePAPR, memory reservation block entries may not be explicitly accessed by the operating system (unless told to elsewhere). The OS may map any reserved entries with cacheable attributes (potentially leading to the issues I described earlier) > Is the semantic really so weak that we cannot use it here? In general, the semantic is too weak. In fact, it's not even strictly defined for the ARM architecture w.r.t. memory attributes, so we have very little guarantee as to what what an OS will do beyond that it will not perform any explicit accesses to the region. In practice, Linux will currently map the region as cacheable, and it may or may not map it shareable depending on SMP/UP (which could be a problem if you want to use a UP Linux to load and kexec an SMP kernel for some reason). It may be that on a given CPU/system implemetation that a memreserve entry is sufficient; but unfortunately this depends on IMPLEMENTATION DEFINED details. Thanks, Mark.
On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > [...] > > > >> This is getting invasive: > > >> > > >> If I add carveouts via adjusting memory banks, I need to account for the > > >> case that an existing bank is split into two halves, creating additional > > >> banks this way. But then current fdt_fixup_memory_banks will no longer > > >> work due to its limitation to the number of physical banks. I could > > >> always add one spare bank to that service, ok, but then the next use > > >> case for carveouts will hit the wall again. So I better double that > > >> limit, or so. > > > > > > Yeah, not fun. > > > > > > If the code is position-independent then you might be able to simply > > > carve out a sufficient proportion from the start of the first entry or > > > the end of the last one, which would avoid splitting. If either of said > > > regions are too small for the monitor code then it's questionable as to > > > whether the OS can make use of it. > > > > The code /seems/ to be position-independent, but locations are so far > > hard-coded in those places that prepare it and move it around. Maybe we > > can decide about the location at runtime, maybe we can simply demand it > > to be at the end or the beginning of some bank. > > If it's possible to do so, it would seem like the nicest option to me. Using the top of memory for this seems like the most natural choice, so I think if we restrict ourselves to that for now we should be fine. The code could probably just get the top of memory from the memory node and adjust it according to the size specified for the secure monitor. Thierry
On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: > On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > > [...] > > > > > >> This is getting invasive: > > > >> > > > >> If I add carveouts via adjusting memory banks, I need to account for the > > > >> case that an existing bank is split into two halves, creating additional > > > >> banks this way. But then current fdt_fixup_memory_banks will no longer > > > >> work due to its limitation to the number of physical banks. I could > > > >> always add one spare bank to that service, ok, but then the next use > > > >> case for carveouts will hit the wall again. So I better double that > > > >> limit, or so. > > > > > > > > Yeah, not fun. > > > > > > > > If the code is position-independent then you might be able to simply > > > > carve out a sufficient proportion from the start of the first entry or > > > > the end of the last one, which would avoid splitting. If either of said > > > > regions are too small for the monitor code then it's questionable as to > > > > whether the OS can make use of it. > > > > > > The code /seems/ to be position-independent, but locations are so far > > > hard-coded in those places that prepare it and move it around. Maybe we > > > can decide about the location at runtime, maybe we can simply demand it > > > to be at the end or the beginning of some bank. > > > > If it's possible to do so, it would seem like the nicest option to me. > > Using the top of memory for this seems like the most natural choice, I think it needs to still be below 4G, doesn't it? So on large mem/LPAE systems some care might be needed. It was suggested by Mark earlier in the thread that this stuff is IMPLEMENTATION DEFINED. Is it possible that we simply don't need to worry about these cross-world cache issues on Tegra? (I must confess that until now I'd assumed that the cache lines were tagged with the world which populated them to stop them interfering with each other in this sort of way...) > so > I think if we restrict ourselves to that for now we should be fine. The > code could probably just get the top of memory from the memory node and > adjust it according to the size specified for the secure monitor. > > Thierry
On 2015-02-19 10:19, Ian Campbell wrote: > On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: >> On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: >>> [...] >>> >>>>>> This is getting invasive: >>>>>> >>>>>> If I add carveouts via adjusting memory banks, I need to account for the >>>>>> case that an existing bank is split into two halves, creating additional >>>>>> banks this way. But then current fdt_fixup_memory_banks will no longer >>>>>> work due to its limitation to the number of physical banks. I could >>>>>> always add one spare bank to that service, ok, but then the next use >>>>>> case for carveouts will hit the wall again. So I better double that >>>>>> limit, or so. >>>>> >>>>> Yeah, not fun. >>>>> >>>>> If the code is position-independent then you might be able to simply >>>>> carve out a sufficient proportion from the start of the first entry or >>>>> the end of the last one, which would avoid splitting. If either of said >>>>> regions are too small for the monitor code then it's questionable as to >>>>> whether the OS can make use of it. >>>> >>>> The code /seems/ to be position-independent, but locations are so far >>>> hard-coded in those places that prepare it and move it around. Maybe we >>>> can decide about the location at runtime, maybe we can simply demand it >>>> to be at the end or the beginning of some bank. >>> >>> If it's possible to do so, it would seem like the nicest option to me. >> >> Using the top of memory for this seems like the most natural choice, > > I think it needs to still be below 4G, doesn't it? So on large mem/LPAE > systems some care might be needed. Argh. That would likely mean we had to split a bank (unless >2G comes in multiple banks), something I'd like to avoid having to implement. > > It was suggested by Mark earlier in the thread that this stuff is > IMPLEMENTATION DEFINED. Is it possible that we simply don't need to > worry about these cross-world cache issues on Tegra? > > (I must confess that until now I'd assumed that the cache lines were > tagged with the world which populated them to stop them interfering with > each other in this sort of way...) I'm pretty sure that is no such thing as a cross-world cache problem. Otherwise the architecture or some implementation would have serious security issues as discussed earlier. To my understanding, Mark's suggestion is now targeting the concern that Linux may accidentally trigger accesses and, thus, stumble or create warnings at least. Jan
On Thu, 2015-02-19 at 10:25 +0100, Jan Kiszka wrote: > On 2015-02-19 10:19, Ian Campbell wrote: > > On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: > >> On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > >>> [...] > >>> > >>>>>> This is getting invasive: > >>>>>> > >>>>>> If I add carveouts via adjusting memory banks, I need to account for the > >>>>>> case that an existing bank is split into two halves, creating additional > >>>>>> banks this way. But then current fdt_fixup_memory_banks will no longer > >>>>>> work due to its limitation to the number of physical banks. I could > >>>>>> always add one spare bank to that service, ok, but then the next use > >>>>>> case for carveouts will hit the wall again. So I better double that > >>>>>> limit, or so. > >>>>> > >>>>> Yeah, not fun. > >>>>> > >>>>> If the code is position-independent then you might be able to simply > >>>>> carve out a sufficient proportion from the start of the first entry or > >>>>> the end of the last one, which would avoid splitting. If either of said > >>>>> regions are too small for the monitor code then it's questionable as to > >>>>> whether the OS can make use of it. > >>>> > >>>> The code /seems/ to be position-independent, but locations are so far > >>>> hard-coded in those places that prepare it and move it around. Maybe we > >>>> can decide about the location at runtime, maybe we can simply demand it > >>>> to be at the end or the beginning of some bank. > >>> > >>> If it's possible to do so, it would seem like the nicest option to me. > >> > >> Using the top of memory for this seems like the most natural choice, > > > > I think it needs to still be below 4G, doesn't it? So on large mem/LPAE > > systems some care might be needed. > > Argh. That would likely mean we had to split a bank (unless >2G comes in > multiple banks), something I'd like to avoid having to implement. I expect it is usual for the 4G boundary to coincide with a bank boundary, even if memory spans the gap -- but I also don't think we can rely on that. > > It was suggested by Mark earlier in the thread that this stuff is > > IMPLEMENTATION DEFINED. Is it possible that we simply don't need to > > worry about these cross-world cache issues on Tegra? > > > > (I must confess that until now I'd assumed that the cache lines were > > tagged with the world which populated them to stop them interfering with > > each other in this sort of way...) > > I'm pretty sure that is no such thing as a cross-world cache problem. > Otherwise the architecture or some implementation would have serious > security issues as discussed earlier. To my understanding, Mark's > suggestion is now targeting the concern that Linux may accidentally > trigger accesses and, thus, stumble or create warnings at least. Ah, then I've misunderstood/misremembered. I think it is fair to say that if a NS-world OS deliberately touches a region marked reserved or not described at all then it deserves whatever fault it gets. But I think what Mark is saying is that just mapping the region but never explicitly accessing it can still result in errors due to e.g. prefetching or speculative behaviour which may use those mappings. IOW it is architecturally allowable for faults arising from accesses never explicitly occurring in the code to result in OS visible faults. Rather than, say, requiring such faults to be squashed until the real/non-speculative access actually really occurs in the program. Ian.
On Thu, Feb 19, 2015 at 10:25:56AM +0100, Jan Kiszka wrote: > On 2015-02-19 10:19, Ian Campbell wrote: > > On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: > >> On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > >>> [...] > >>> > >>>>>> This is getting invasive: > >>>>>> > >>>>>> If I add carveouts via adjusting memory banks, I need to account for the > >>>>>> case that an existing bank is split into two halves, creating additional > >>>>>> banks this way. But then current fdt_fixup_memory_banks will no longer > >>>>>> work due to its limitation to the number of physical banks. I could > >>>>>> always add one spare bank to that service, ok, but then the next use > >>>>>> case for carveouts will hit the wall again. So I better double that > >>>>>> limit, or so. > >>>>> > >>>>> Yeah, not fun. > >>>>> > >>>>> If the code is position-independent then you might be able to simply > >>>>> carve out a sufficient proportion from the start of the first entry or > >>>>> the end of the last one, which would avoid splitting. If either of said > >>>>> regions are too small for the monitor code then it's questionable as to > >>>>> whether the OS can make use of it. > >>>> > >>>> The code /seems/ to be position-independent, but locations are so far > >>>> hard-coded in those places that prepare it and move it around. Maybe we > >>>> can decide about the location at runtime, maybe we can simply demand it > >>>> to be at the end or the beginning of some bank. > >>> > >>> If it's possible to do so, it would seem like the nicest option to me. > >> > >> Using the top of memory for this seems like the most natural choice, > > > > I think it needs to still be below 4G, doesn't it? So on large mem/LPAE > > systems some care might be needed. > > Argh. That would likely mean we had to split a bank (unless >2G comes in > multiple banks), something I'd like to avoid having to implement. So if we wanted to (properly) support systems with more than 2 GiB of RAM we'd need to make U-Boot LPAE aware. That would cause other kinds of problems, though, and I'm not sure that it's even possible. Looking at the register sets involved it seems like we can't have a reset vector pointing at anything beyond the 4 GiB boundary anyway. It would also mean that a bunch of things might break since I think there are many occurrences in U-Boot where pointers are assumed to be 32-bit. Perhaps a compromise for now would be to truncate the amount of available memory to 2 GiB if PSCI is used? That's not really optimal but should give us something to experiment. I don't think any of the boards supported upstream have more than 2 GiB, though I'm sure there will be in the not-so-distant future. Most of those may be 64-bit ARM, though, in which case the problem goes away somewhat. Unfortunately it seems like even for 64-bit SoCs the 32-bit registers for the reset vectors remain, so it would seem like at least for Tegra we're going to need code to split up the memory node eventually. Either that or we move the monitor to the beginning of RAM. But I don't think we can do that either because the Linux kernel will decompress to an offset of 0x00008000 after the start of RAM. Thierry
On Tue, Feb 17, 2015 at 09:09:57AM +0100, Jan Kiszka wrote: > On 2015-02-16 16:38, Jan Kiszka wrote: > > On 2015-02-16 15:56, Mark Rutland wrote: > >> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: > >>> On 2015-02-16 15:25, Mark Rutland wrote: > >>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: > >>>>> On 2015-02-16 14:42, Mark Rutland wrote: > >>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: > >>>>>>> From: Ian Campbell <ijc@hellion.org.uk> > >>>>>>> > >>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but > >>>>>>> it has been relocated, so the reservation of __secure_start does not apply. > >>>>>>> > >>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a > >>>>>>> region. > >>>>>>> > >>>>>>> This will be used in a subsequent patch for Jetson-TK1 > >>>>>> > >>>>>> Using a memreserve and allowing the OS to map the memory but not poke it > >>>>>> can be problematic due to the potential of mismatched attributes between > >>>>>> the monitor and the OS. > >>>>> > >>>>> OK, here my knowledge is not yet sufficient to process this remark. What > >>>>> kind of problems can arise from what kind of attribute mismatch? And why > >>>>> should the OS be able to cause problems for the monitor? > >>>> > >>>> For example, consider the case of the region being mapped cacheable by > >>>> the OS but not by the monitor. The monitor communicates between cores > >>>> expecting to never hit in a cache (because it uses a non-cacheable > >>>> mapping), but the mapping used by the OS can cause the region to be > >>>> allocated into caches at any point in time even if it never accesses the > >>>> region explicitly. > >>>> > >>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this > >>>> is called an "unexepcted data cache hit"), so the cache allocations > >>>> caused by the OS can mask data other CPUs wrote straight to memory. > >>>> > >>>> Other than that case, I believe the rules given in the ARM ARM for > >>>> mismatched memory attributes may apply for similar reasons. Thus > >>>> allowing the OS to map this memory can cause a loss of coherency on the > >>>> monitor side, if the OS and monitor map the region with different > >>>> attributes. > >>>> > >>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the > >>>> system you're dealing with. I don't immediately know whether that is the > >>>> case, however. Never telling the OS about the memory in the first place > >>>> avoids the possibility in all cases. > >>> > >>> But from a security point of view, it must not matter if the OS maps the > >>> memory or not - the monitor must be robust against that, no? If the > >>> architecture cannot provide such guarantees, it has to be worked around > >>> in software in the monitor (I hope you can do so...). > >> > >> Well, yes and no. > >> > >> In this case it sounds like due to the security controller you should > >> never encounter the mismatched attributes issue in the first place, > >> though you may encounter issues w.r.t. speculative accesses triggering > >> violations arbitrarily. Not telling the OS about the secure memory means > >> that said violations shouldn't occur in normal operation; only when the > >> non-secure OS is trying to do something bad. > >> > >> If the OS has access to the memory, then you're already trusting it to > >> not write to there or you can't trust that memory at all (and hence > >> cannot use it). Given that means you must already assume that the OS is > >> cooperative, it's simpler to not tell it about the memory than to add > >> cache maintenance around every memory access within the monitor. You can > >> never make things secure in this case, but you can at least offer the > >> abstraction provided by PSCI. > >> > >> So as far as I can see in either case it's better to not tell the OS > >> about the memory you wish to use from the monitor. If you have no HW > >> protection and can't trust the OS then you've already lost, and if you > >> do have HW protection you don't want it to trigger > >> continuously/spuriously as a result of speculation. > > > > OK, that makes sense again. > > > > Now I just need to figure out how to split/adjust the memory node > > instead of adding a reservation region. > > This is getting invasive: > > If I add carveouts via adjusting memory banks, I need to account for the > case that an existing bank is split into two halves, creating additional > banks this way. But then current fdt_fixup_memory_banks will no longer > work due to its limitation to the number of physical banks. I could > always add one spare bank to that service, ok, but then the next use > case for carveouts will hit the wall again. So I better double that > limit, or so. fdt_fixup_memory_banks() will shout if it doesn't have enough banks, so I suppose we could put that problem off to the configuration files. For example we could add something like: #ifdef CONFIG_ARMV7_PSCI # define CONFIG_NR_DRAM_BANKS 2 #else # define CONFIG_NR_DRAM_BANKS 1 #endif to tegra-common.h and make sure to define CONFIG_ARMV7_PSCI before that is included. That could easily be extended using something like the following if you're concerned about there being many carveout use-cases: #ifdef CONFIG_ARMV7_PSCI # define PSCI_EXTRA_DRAM_BANKS 1 #else # define PSCI_EXTRA_DRAM_BANKS 0 #endif #ifdef CONFIG_FOO # define FOO_EXTRA_DRAM_BANKS 1 #else # define FOO_EXTRA_DRAM_BANKS 0 #endif #define CONFIG_NR_DRAM_BANKS (1 + PSCI_EXTRA_DRAM_BANKS + FOO_EXTRA_DRAM_BANKS) But I think it'd be good enough for now to go with the first snippet. Thierry
On 2015-02-19 11:34, Thierry Reding wrote: > On Tue, Feb 17, 2015 at 09:09:57AM +0100, Jan Kiszka wrote: >> On 2015-02-16 16:38, Jan Kiszka wrote: >>> On 2015-02-16 15:56, Mark Rutland wrote: >>>> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote: >>>>> On 2015-02-16 15:25, Mark Rutland wrote: >>>>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote: >>>>>>> On 2015-02-16 14:42, Mark Rutland wrote: >>>>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote: >>>>>>>>> From: Ian Campbell <ijc@hellion.org.uk> >>>>>>>>> >>>>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but >>>>>>>>> it has been relocated, so the reservation of __secure_start does not apply. >>>>>>>>> >>>>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a >>>>>>>>> region. >>>>>>>>> >>>>>>>>> This will be used in a subsequent patch for Jetson-TK1 >>>>>>>> >>>>>>>> Using a memreserve and allowing the OS to map the memory but not poke it >>>>>>>> can be problematic due to the potential of mismatched attributes between >>>>>>>> the monitor and the OS. >>>>>>> >>>>>>> OK, here my knowledge is not yet sufficient to process this remark. What >>>>>>> kind of problems can arise from what kind of attribute mismatch? And why >>>>>>> should the OS be able to cause problems for the monitor? >>>>>> >>>>>> For example, consider the case of the region being mapped cacheable by >>>>>> the OS but not by the monitor. The monitor communicates between cores >>>>>> expecting to never hit in a cache (because it uses a non-cacheable >>>>>> mapping), but the mapping used by the OS can cause the region to be >>>>>> allocated into caches at any point in time even if it never accesses the >>>>>> region explicitly. >>>>>> >>>>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this >>>>>> is called an "unexepcted data cache hit"), so the cache allocations >>>>>> caused by the OS can mask data other CPUs wrote straight to memory. >>>>>> >>>>>> Other than that case, I believe the rules given in the ARM ARM for >>>>>> mismatched memory attributes may apply for similar reasons. Thus >>>>>> allowing the OS to map this memory can cause a loss of coherency on the >>>>>> monitor side, if the OS and monitor map the region with different >>>>>> attributes. >>>>>> >>>>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the >>>>>> system you're dealing with. I don't immediately know whether that is the >>>>>> case, however. Never telling the OS about the memory in the first place >>>>>> avoids the possibility in all cases. >>>>> >>>>> But from a security point of view, it must not matter if the OS maps the >>>>> memory or not - the monitor must be robust against that, no? If the >>>>> architecture cannot provide such guarantees, it has to be worked around >>>>> in software in the monitor (I hope you can do so...). >>>> >>>> Well, yes and no. >>>> >>>> In this case it sounds like due to the security controller you should >>>> never encounter the mismatched attributes issue in the first place, >>>> though you may encounter issues w.r.t. speculative accesses triggering >>>> violations arbitrarily. Not telling the OS about the secure memory means >>>> that said violations shouldn't occur in normal operation; only when the >>>> non-secure OS is trying to do something bad. >>>> >>>> If the OS has access to the memory, then you're already trusting it to >>>> not write to there or you can't trust that memory at all (and hence >>>> cannot use it). Given that means you must already assume that the OS is >>>> cooperative, it's simpler to not tell it about the memory than to add >>>> cache maintenance around every memory access within the monitor. You can >>>> never make things secure in this case, but you can at least offer the >>>> abstraction provided by PSCI. >>>> >>>> So as far as I can see in either case it's better to not tell the OS >>>> about the memory you wish to use from the monitor. If you have no HW >>>> protection and can't trust the OS then you've already lost, and if you >>>> do have HW protection you don't want it to trigger >>>> continuously/spuriously as a result of speculation. >>> >>> OK, that makes sense again. >>> >>> Now I just need to figure out how to split/adjust the memory node >>> instead of adding a reservation region. >> >> This is getting invasive: >> >> If I add carveouts via adjusting memory banks, I need to account for the >> case that an existing bank is split into two halves, creating additional >> banks this way. But then current fdt_fixup_memory_banks will no longer >> work due to its limitation to the number of physical banks. I could >> always add one spare bank to that service, ok, but then the next use >> case for carveouts will hit the wall again. So I better double that >> limit, or so. > > fdt_fixup_memory_banks() will shout if it doesn't have enough banks, so > I suppose we could put that problem off to the configuration files. For > example we could add something like: > > #ifdef CONFIG_ARMV7_PSCI > # define CONFIG_NR_DRAM_BANKS 2 > #else > # define CONFIG_NR_DRAM_BANKS 1 > #endif > > to tegra-common.h and make sure to define CONFIG_ARMV7_PSCI before that > is included. That could easily be extended using something like the > following if you're concerned about there being many carveout use-cases: > > #ifdef CONFIG_ARMV7_PSCI > # define PSCI_EXTRA_DRAM_BANKS 1 > #else > # define PSCI_EXTRA_DRAM_BANKS 0 > #endif > > #ifdef CONFIG_FOO > # define FOO_EXTRA_DRAM_BANKS 1 > #else > # define FOO_EXTRA_DRAM_BANKS 0 > #endif > > #define CONFIG_NR_DRAM_BANKS (1 + > PSCI_EXTRA_DRAM_BANKS + > FOO_EXTRA_DRAM_BANKS) > > But I think it'd be good enough for now to go with the first snippet. Well, I rather hope we can get away with v3 from the time being, i.e. enforced beginning/end of bank. Jan
On Thu, Feb 19, 2015 at 09:25:56AM +0000, Jan Kiszka wrote: > On 2015-02-19 10:19, Ian Campbell wrote: > > On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: > >> On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > >>> [...] > >>> > >>>>>> This is getting invasive: > >>>>>> > >>>>>> If I add carveouts via adjusting memory banks, I need to account for the > >>>>>> case that an existing bank is split into two halves, creating additional > >>>>>> banks this way. But then current fdt_fixup_memory_banks will no longer > >>>>>> work due to its limitation to the number of physical banks. I could > >>>>>> always add one spare bank to that service, ok, but then the next use > >>>>>> case for carveouts will hit the wall again. So I better double that > >>>>>> limit, or so. > >>>>> > >>>>> Yeah, not fun. > >>>>> > >>>>> If the code is position-independent then you might be able to simply > >>>>> carve out a sufficient proportion from the start of the first entry or > >>>>> the end of the last one, which would avoid splitting. If either of said > >>>>> regions are too small for the monitor code then it's questionable as to > >>>>> whether the OS can make use of it. > >>>> > >>>> The code /seems/ to be position-independent, but locations are so far > >>>> hard-coded in those places that prepare it and move it around. Maybe we > >>>> can decide about the location at runtime, maybe we can simply demand it > >>>> to be at the end or the beginning of some bank. > >>> > >>> If it's possible to do so, it would seem like the nicest option to me. > >> > >> Using the top of memory for this seems like the most natural choice, > > > > I think it needs to still be below 4G, doesn't it? So on large mem/LPAE > > systems some care might be needed. > > Argh. That would likely mean we had to split a bank (unless >2G comes in > multiple banks), something I'd like to avoid having to implement. > > > > > It was suggested by Mark earlier in the thread that this stuff is > > IMPLEMENTATION DEFINED. Is it possible that we simply don't need to > > worry about these cross-world cache issues on Tegra? > > > > (I must confess that until now I'd assumed that the cache lines were > > tagged with the world which populated them to stop them interfering with > > each other in this sort of way...) > > I'm pretty sure that is no such thing as a cross-world cache problem. > Otherwise the architecture or some implementation would have serious > security issues as discussed earlier. To my understanding, Mark's > suggestion is now targeting the concern that Linux may accidentally > trigger accesses and, thus, stumble or create warnings at least. Yup. If the memory is protected by some configurable security controller (as seems to be the case on Tegra), the non-secure side accessing any memory protected by it will result in a violation (and presumably bring down the non-secure world). We need to prevent speculative accesses (the security controller can't tell the difference), and therefore cannot map the memory at all (so a /memreserve/ is insufficient). Depending on implementation details there are other potential problems, and carving out the memory explicitly solves all that I am aware of without having to rely on implementation-specific details. Thanks, Mark.
On Thu, Feb 19, 2015 at 10:13:58AM +0000, Ian Campbell wrote: > On Thu, 2015-02-19 at 10:25 +0100, Jan Kiszka wrote: > > On 2015-02-19 10:19, Ian Campbell wrote: > > > On Thu, 2015-02-19 at 09:28 +0100, Thierry Reding wrote: > > >> On Tue, Feb 17, 2015 at 11:55:24AM +0000, Mark Rutland wrote: > > >>> [...] > > >>> > > >>>>>> This is getting invasive: > > >>>>>> > > >>>>>> If I add carveouts via adjusting memory banks, I need to account for the > > >>>>>> case that an existing bank is split into two halves, creating additional > > >>>>>> banks this way. But then current fdt_fixup_memory_banks will no longer > > >>>>>> work due to its limitation to the number of physical banks. I could > > >>>>>> always add one spare bank to that service, ok, but then the next use > > >>>>>> case for carveouts will hit the wall again. So I better double that > > >>>>>> limit, or so. > > >>>>> > > >>>>> Yeah, not fun. > > >>>>> > > >>>>> If the code is position-independent then you might be able to simply > > >>>>> carve out a sufficient proportion from the start of the first entry or > > >>>>> the end of the last one, which would avoid splitting. If either of said > > >>>>> regions are too small for the monitor code then it's questionable as to > > >>>>> whether the OS can make use of it. > > >>>> > > >>>> The code /seems/ to be position-independent, but locations are so far > > >>>> hard-coded in those places that prepare it and move it around. Maybe we > > >>>> can decide about the location at runtime, maybe we can simply demand it > > >>>> to be at the end or the beginning of some bank. > > >>> > > >>> If it's possible to do so, it would seem like the nicest option to me. > > >> > > >> Using the top of memory for this seems like the most natural choice, > > > > > > I think it needs to still be below 4G, doesn't it? So on large mem/LPAE > > > systems some care might be needed. > > > > Argh. That would likely mean we had to split a bank (unless >2G comes in > > multiple banks), something I'd like to avoid having to implement. > > I expect it is usual for the 4G boundary to coincide with a bank > boundary, even if memory spans the gap -- but I also don't think we can > rely on that. > > > > It was suggested by Mark earlier in the thread that this stuff is > > > IMPLEMENTATION DEFINED. Is it possible that we simply don't need to > > > worry about these cross-world cache issues on Tegra? > > > > > > (I must confess that until now I'd assumed that the cache lines were > > > tagged with the world which populated them to stop them interfering with > > > each other in this sort of way...) > > > > I'm pretty sure that is no such thing as a cross-world cache problem. > > Otherwise the architecture or some implementation would have serious > > security issues as discussed earlier. To my understanding, Mark's > > suggestion is now targeting the concern that Linux may accidentally > > trigger accesses and, thus, stumble or create warnings at least. > > Ah, then I've misunderstood/misremembered. > > I think it is fair to say that if a NS-world OS deliberately touches a > region marked reserved or not described at all then it deserves whatever > fault it gets. But I think what Mark is saying is that just mapping the > region but never explicitly accessing it can still result in errors due > to e.g. prefetching or speculative behaviour which may use those > mappings. In this case the external memory security controller block likely can't tell the difference (speculative and explicit accesses will look the same on the bus). So any speculative access can trigger violations and bring down the non-secure world (or just DoS the secure world if it signals the secure world but provides a dummy result to non-secure reads). > IOW it is architecturally allowable for faults arising from accesses > never explicitly occurring in the code to result in OS visible faults. > Rather than, say, requiring such faults to be squashed until the > real/non-speculative access actually really occurs in the program. Yes. Anything mapped cacheable (or non-executable) can be speculatively read, and if those accesses trigger some error on the bus (e.g. due to a security controller), you'll get some kind of asynchronous abort reported by the CPU. Thanks, Mark.
diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c index ad19e4c..eb95031 100644 --- a/arch/arm/cpu/armv7/virt-dt.c +++ b/arch/arm/cpu/armv7/virt-dt.c @@ -96,6 +96,11 @@ int armv7_update_dt(void *fdt) /* secure code lives in RAM, keep it alive */ fdt_add_mem_rsv(fdt, (unsigned long)__secure_start, __secure_end - __secure_start); +#elif defined(CONFIG_ARMV7_SECURE_RESERVE_SIZE) + /* secure code has been relocated into RAM carveout, keep it alive */ + fdt_add_mem_rsv(fdt, + CONFIG_ARMV7_SECURE_BASE, + CONFIG_ARMV7_SECURE_RESERVE_SIZE); #endif return fdt_psci(fdt);