Does nothing as default, arch can implement their function to map
fdt to RO/RW. This is convenient if arch map fdt to RO during init
but needs to write fdt in some special cases after that.
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/of/fdt.c | 13 +++++++++++++
include/linux/of_fdt.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index de893c9616a1..e84971d1e9ea 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -30,6 +30,19 @@
#include "of_private.h"
+/*
+ * update_fdt_pgprot - Arch hook for changing fdt pgprot
+ *
+ * @prot: page protection flags for fdt
+ *
+ * Architecture can implement this function if they want to chagne
+ * fdt page protection flags before or after doing modification and
+ * fixups to fdt.
+ *
+ * Default does nothing.
+ */
+__weak void update_fdt_pgprot(pgprot_t prot) {}
+
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index a713e5d156d8..406c3e7b2b75 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -109,5 +109,7 @@ static inline void unflatten_device_tree(void) {}
static inline void unflatten_and_copy_device_tree(void) {}
#endif /* CONFIG_OF_EARLY_FLATTREE */
+extern void update_fdt_pgprot(pgprot_t prot);
+
#endif /* __ASSEMBLY__ */
#endif /* _LINUX_OF_FDT_H */
--
2.20.1
Basically does similar things like __fixmap_remap_fdt(). It's supposed
to be called after fixmap_remap_fdt() is called at least once, so region
checking can be skipped. Since it needs to know dt physical address, make
a copy of the value of __fdt_pointer.
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
arch/arm64/kernel/setup.c | 2 ++
arch/arm64/mm/mmu.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 413d566405d1..207cbb5f7965 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -66,6 +66,7 @@ static int num_standard_resources;
static struct resource *standard_resources;
phys_addr_t __fdt_pointer __initdata;
+phys_addr_t fdt_pointer;
/*
* Standard memory resources
@@ -292,6 +293,7 @@ void __init setup_arch(char **cmdline_p)
early_fixmap_init();
early_ioremap_init();
+ fdt_pointer = __fdt_pointer;
setup_machine_fdt(__fdt_pointer);
parse_early_param();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index a170c6369a68..196ab4d9e92a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -32,6 +32,7 @@
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/vmalloc.h>
+#include <linux/of_fdt.h>
#include <asm/barrier.h>
#include <asm/cputype.h>
@@ -953,6 +954,22 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys)
return dt_virt;
}
+extern phys_addr_t fdt_pointer;
+
+/* Should be called after fixmap_remap_fdt() is called. */
+void update_fdt_pgprot(pgprot_t prot)
+{
+ u64 dt_virt_base = __fix_to_virt(FIX_FDT);
+ int offset, size;
+
+ offset = fdt_pointer % SWAPPER_BLOCK_SIZE;
+ size = fdt_totalsize((void *)dt_virt_base + offset);
+
+ update_mapping_prot(round_down(fdt_pointer, SWAPPER_BLOCK_SIZE),
+ dt_virt_base,
+ round_up(offset + size, SWAPPER_BLOCK_SIZE), prot);
+}
+
int __init arch_ioremap_pud_supported(void)
{
/* only 4k granule supports level 1 block mappings */
--
2.20.1
Introducing a chosen node, rng-seed, which is an entropy that can be
passed to kernel called very early to increase initial device
randomness. Bootloader should provide this entropy and the value is
read from /chosen/rng-seed in DT.
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
change v2->v3:
1. use arch hook for fdt pgprot change
2. handle CONFIG_KEXEC
---
Documentation/devicetree/bindings/chosen.txt | 14 +++++
drivers/of/fdt.c | 55 ++++++++++++++++++++
2 files changed, 69 insertions(+)
diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..fef5c82672dc 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be overwritten by
the Linux EFI stub (which will populate the property itself, using
EFI_RNG_PROTOCOL).
+rng-seed
+-----------
+
+This property served as an entropy to add device randomness. It is parsed
+as a byte array, e.g.
+
+/ {
+ chosen {
+ rng-seed = <0x31 0x95 0x1b 0x3c 0xc9 0xfa 0xb3 ...>;
+ };
+};
+
+This random value should be provided by bootloader.
+
stdout-path
-----------
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e84971d1e9ea..e9862657268d 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -10,6 +10,7 @@
#include <linux/crc32.h>
#include <linux/kernel.h>
+#include <linux/kexec.h>
#include <linux/initrd.h>
#include <linux/memblock.h>
#include <linux/mutex.h>
@@ -24,6 +25,8 @@
#include <linux/debugfs.h>
#include <linux/serial_core.h>
#include <linux/sysfs.h>
+#include <linux/random.h>
+#include <linux/reboot.h>
#include <asm/setup.h> /* for COMMAND_LINE_SIZE */
#include <asm/page.h>
@@ -1087,11 +1090,14 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
return 0;
}
+int rng_seed_size;
+
int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data)
{
int l;
const char *p;
+ const void *rng_seed;
pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
@@ -1126,6 +1132,16 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
pr_debug("Command line is: %s\n", (char*)data);
+ rng_seed = of_get_flat_dt_prop(node, "rng-seed", &rng_seed_size);
+ if (rng_seed && rng_seed_size > 0) {
+ add_device_randomness(rng_seed, rng_seed_size);
+
+ /* try to clear seed so it won't be found. */
+ update_fdt_pgprot(PAGE_KERNEL);
+ fdt_delprop(initial_boot_params, node, "rng-seed");
+ update_fdt_pgprot(PAGE_KERNEL_RO);
+ }
+
/* break now */
return 1;
}
@@ -1327,4 +1343,43 @@ static int __init of_fdt_raw_init(void)
late_initcall(of_fdt_raw_init);
#endif
+#ifdef CONFIG_KEXEC
+static int update_fdt_random_seed(struct notifier_block *nb,
+ unsigned long code, void *unused)
+{
+ int node;
+ void *rng_seed;
+
+ if (!kexec_in_progress || !rng_seed_size)
+ return NOTIFY_DONE;
+
+ node = fdt_path_offset(initial_boot_params, "/chosen");
+ if (node < 0)
+ node = fdt_path_offset(initial_boot_params, "/chosen@0");
+ if (node < 0)
+ return NOTIFY_DONE;
+
+ rng_seed = kmalloc(rng_seed_size, GFP_ATOMIC);
+ get_random_bytes(rng_seed, rng_seed_size);
+
+ update_fdt_pgprot(PAGE_KERNEL);
+ fdt_setprop(initial_boot_params, node, "rng-seed", rng_seed,
+ rng_seed_size);
+
+ kfree(rng_seed);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block fdt_random_seed_nb = {
+ .notifier_call = update_fdt_random_seed,
+};
+
+static int register_update_fdt_random_seed(void)
+{
+ return register_reboot_notifier(&fdt_random_seed_nb);
+}
+late_initcall(register_update_fdt_random_seed);
+#endif
+
#endif /* CONFIG_OF_EARLY_FLATTREE */
--
2.20.1
On Thu, May 16, 2019 at 5:28 AM Hsin-Yi Wang <[email protected]> wrote:
>
> Basically does similar things like __fixmap_remap_fdt(). It's supposed
> to be called after fixmap_remap_fdt() is called at least once, so region
> checking can be skipped. Since it needs to know dt physical address, make
> a copy of the value of __fdt_pointer.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
> ---
> arch/arm64/kernel/setup.c | 2 ++
> arch/arm64/mm/mmu.c | 17 +++++++++++++++++
> 2 files changed, 19 insertions(+)
Why not just map the FDT R/W at the start and change it to RO just
before calling unflatten_device_tree? Then all the FDT scanning
functions or any future fixups we need can just assume R/W. That is
essentially what Stephen suggested. However, there's no need for a
weak function as it can all be done within the arch code.
However, I'm still wondering why the FDT needs to be RO in the first place.
Rob
On Thu, 16 May 2019 at 16:37, Rob Herring <[email protected]> wrote:
>
> On Thu, May 16, 2019 at 5:28 AM Hsin-Yi Wang <[email protected]> wrote:
> >
> > Basically does similar things like __fixmap_remap_fdt(). It's supposed
> > to be called after fixmap_remap_fdt() is called at least once, so region
> > checking can be skipped. Since it needs to know dt physical address, make
> > a copy of the value of __fdt_pointer.
> >
> > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > ---
> > arch/arm64/kernel/setup.c | 2 ++
> > arch/arm64/mm/mmu.c | 17 +++++++++++++++++
> > 2 files changed, 19 insertions(+)
>
> Why not just map the FDT R/W at the start and change it to RO just
> before calling unflatten_device_tree? Then all the FDT scanning
> functions or any future fixups we need can just assume R/W. That is
> essentially what Stephen suggested. However, there's no need for a
> weak function as it can all be done within the arch code.
>
> However, I'm still wondering why the FDT needs to be RO in the first place.
>
It was RO because it could be RO, and we wanted to ensure that it
didn't get modified inadvertently (hence the CRC check we added as
well)
If there is a need for the FDT to be RW, let's make it RW.
On Thu, May 16, 2019 at 09:37:05AM -0500, Rob Herring wrote:
> On Thu, May 16, 2019 at 5:28 AM Hsin-Yi Wang <[email protected]> wrote:
> >
> > Basically does similar things like __fixmap_remap_fdt(). It's supposed
> > to be called after fixmap_remap_fdt() is called at least once, so region
> > checking can be skipped. Since it needs to know dt physical address, make
> > a copy of the value of __fdt_pointer.
> >
> > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > ---
> > arch/arm64/kernel/setup.c | 2 ++
> > arch/arm64/mm/mmu.c | 17 +++++++++++++++++
> > 2 files changed, 19 insertions(+)
>
> Why not just map the FDT R/W at the start and change it to RO just
> before calling unflatten_device_tree? Then all the FDT scanning
> functions or any future fixups we need can just assume R/W. That is
> essentially what Stephen suggested. However, there's no need for a
> weak function as it can all be done within the arch code.
>
> However, I'm still wondering why the FDT needs to be RO in the first place.
We want to preserve the original FDT in a pristine form for kexec (and
when exposed to userspace), and mapping it RO was the easiest way to
catch it being randomly modified (e.g. without fixups applied).
I'd prefer to keep it RO once we've removed/cleared certain properties
from the chosen node that don't make sense to pass on for kexec
Thanks,
Mark.
On Thu, May 16, 2019 at 10:37 PM Rob Herring <[email protected]> wrote:
>
> Why not just map the FDT R/W at the start and change it to RO just
> before calling unflatten_device_tree? Then all the FDT scanning
> functions or any future fixups we need can just assume R/W. That is
> essentially what Stephen suggested. However, there's no need for a
> weak function as it can all be done within the arch code.
>
We need to add a new seed for kexec
On Thu, May 16, 2019 at 9:43 AM Mark Rutland <[email protected]> wrote:
>
> On Thu, May 16, 2019 at 09:37:05AM -0500, Rob Herring wrote:
> > On Thu, May 16, 2019 at 5:28 AM Hsin-Yi Wang <[email protected]> wrote:
> > >
> > > Basically does similar things like __fixmap_remap_fdt(). It's supposed
> > > to be called after fixmap_remap_fdt() is called at least once, so region
> > > checking can be skipped. Since it needs to know dt physical address, make
> > > a copy of the value of __fdt_pointer.
> > >
> > > Signed-off-by: Hsin-Yi Wang <[email protected]>
> > > ---
> > > arch/arm64/kernel/setup.c | 2 ++
> > > arch/arm64/mm/mmu.c | 17 +++++++++++++++++
> > > 2 files changed, 19 insertions(+)
> >
> > Why not just map the FDT R/W at the start and change it to RO just
> > before calling unflatten_device_tree? Then all the FDT scanning
> > functions or any future fixups we need can just assume R/W. That is
> > essentially what Stephen suggested. However, there's no need for a
> > weak function as it can all be done within the arch code.
> >
> > However, I'm still wondering why the FDT needs to be RO in the first place.
>
> We want to preserve the original FDT in a pristine form for kexec (and
> when exposed to userspace), and mapping it RO was the easiest way to
> catch it being randomly modified (e.g. without fixups applied).
The CRC check already existed for this purpose and that works for
every arch including ones where the FDT is copied.
BTW, This version of the patchset disables the export to userspace
since the CRC will be wrong.
> I'd prefer to keep it RO once we've removed/cleared certain properties
> from the chosen node that don't make sense to pass on for kexec
I want clear rules about when the FDT can be modified or not which are
not arch specific.
It's really only a question of with what granularity it's made R/W.
Wrapping every modification seems like overkill to me.
Rob
On Thu, May 16, 2019 at 11:32 PM Rob Herring <[email protected]> wrote:
> Doesn't kexec operate on a copy because it already does modifications.
>
Hi Rob,
This patch is to assist "[PATCH v3 3/3] fdt: add support for rng-seed"
(https://lkml.org/lkml/2019/5/16/257). I thought that by default
second kernel would use original fdt, so I write new seed back to
original fdt. Might be wrong.
** "[PATCH v3 3/3] fdt: add support for rng-seed" is supposed to
handle for adding new seed in kexec case, discussed in v2
(https://lkml.org/lkml/2019/5/13/425)
By default (not considering user defines their own fdt), if second
kernel uses copied fdt, when is it copied and can we modify that?
Thanks!
On 5/16/19 3:28 AM, Hsin-Yi Wang wrote:
> Introducing a chosen node, rng-seed, which is an entropy that can be
> passed to kernel called very early to increase initial device
> randomness. Bootloader should provide this entropy and the value is
> read from /chosen/rng-seed in DT.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>
> ---
> change v2->v3:
> 1. use arch hook for fdt pgprot change
> 2. handle CONFIG_KEXEC
> ---
> Documentation/devicetree/bindings/chosen.txt | 14 +++++
> drivers/of/fdt.c | 55 ++++++++++++++++++++
> 2 files changed, 69 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646..fef5c82672dc 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -28,6 +28,20 @@ mode) when EFI_RNG_PROTOCOL is supported, it will be overwritten by
> the Linux EFI stub (which will populate the property itself, using
> EFI_RNG_PROTOCOL).
>
> +rng-seed
> +-----------
> +
> +This property served as an entropy to add device randomness. It is parsed
serves
> +as a byte array, e.g.
> +
> +/ {
> + chosen {
> + rng-seed = <0x31 0x95 0x1b 0x3c 0xc9 0xfa 0xb3 ...>;
> + };
> +};
> +
> +This random value should be provided by bootloader.
> +
> stdout-path
> -----------
>
--
~Randy
On Thu, May 16, 2019 at 9:51 AM Hsin-Yi Wang <[email protected]> wrote:
>
> On Thu, May 16, 2019 at 10:37 PM Rob Herring <[email protected]> wrote:
>
> >
> > Why not just map the FDT R/W at the start and change it to RO just
> > before calling unflatten_device_tree? Then all the FDT scanning
> > functions or any future fixups we need can just assume R/W. That is
> > essentially what Stephen suggested. However, there's no need for a
> > weak function as it can all be done within the arch code.
> >
> We need to add a new seed for kexec
Doesn't kexec operate on a copy because it already does modifications.
Rob
Quoting James Morse (2019-05-16 10:34:16)
> Hi!
>
> On 16/05/2019 17:48, Hsin-Yi Wang wrote:
> > On Thu, May 16, 2019 at 11:32 PM Rob Herring <[email protected]> wrote:
> >> Doesn't kexec operate on a copy because it already does modifications.
>
> It does!
>
> > This patch is to assist "[PATCH v3 3/3] fdt: add support for rng-seed"
> > (https://lkml.org/lkml/2019/5/16/257). I thought that by default
> > second kernel would use original fdt, so I write new seed back to
> > original fdt. Might be wrong.
> >
> > ** "[PATCH v3 3/3] fdt: add support for rng-seed" is supposed to
> > handle for adding new seed in kexec case, discussed in v2
> > (https://lkml.org/lkml/2019/5/13/425)
> >
> > By default (not considering user defines their own fdt), if second
> > kernel uses copied fdt, when is it copied and can we modify that?
>
> Regular kexec's user-space already updates the dtb for the cmdline and maybe the initrd.
> For KASLR, it generates its own seed with getrandom():
>
> https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/arm64/kexec-arm64.c#n483
>
> If user-space can do it, user-space should do it!
>
Doesn't it need to be done in two places? Userspace and also in the
kernel when kexec_file_load() is used? At least, I see a bit of code
that does kaslr seed updates to the copied dtb in setup_dtb() of
arch/arm64/kernel/machine_kexec_file.c that probably needs to get an
update for this new property too.
Hi!
On 16/05/2019 17:48, Hsin-Yi Wang wrote:
> On Thu, May 16, 2019 at 11:32 PM Rob Herring <[email protected]> wrote:
>> Doesn't kexec operate on a copy because it already does modifications.
It does!
> This patch is to assist "[PATCH v3 3/3] fdt: add support for rng-seed"
> (https://lkml.org/lkml/2019/5/16/257). I thought that by default
> second kernel would use original fdt, so I write new seed back to
> original fdt. Might be wrong.
>
> ** "[PATCH v3 3/3] fdt: add support for rng-seed" is supposed to
> handle for adding new seed in kexec case, discussed in v2
> (https://lkml.org/lkml/2019/5/13/425)
>
> By default (not considering user defines their own fdt), if second
> kernel uses copied fdt, when is it copied and can we modify that?
Regular kexec's user-space already updates the dtb for the cmdline and maybe the initrd.
For KASLR, it generates its own seed with getrandom():
https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/arm64/kexec-arm64.c#n483
If user-space can do it, user-space should do it!
Thanks,
James