fdt: arch/arm64: Delete the rng-seed property after use
Subsystems (2)
PATCH 1/1 fdt: arch/arm64: Delete the rng-seed property after use
As a part of platform boot, device tree is being read to extract randonmess bits. The 'rng-seed' property is used for that purpose. After reading the value, the field was overridden with NOP instead of being deleted or zeroed. The problem is that NOPed fields are later not reused, and kexec code appended this property every time DTB is prepared:
/* add rng-seed */
if (rng_is_initialized()) {
void *rng_seed;
ret = fdt_setprop_placeholder(dtb, off, FDT_PROP_RNG_SEED,
RNG_SEED_SIZE, &rng_seed);
if (ret)
goto out;
get_random_bytes(rng_seed, RNG_SEED_SIZE); }
(source: arch/arm64/kernel/machine_kexec_file.c)
Taken together, DTB grew at each kexec by 140 bytes ie. size of the newly added (and not overwritten) rng-seed property. ARM64 sets a hard limit on FDT size at 2MB, which means that after at most 14,979 kexecs DTB exceeded the limit causing catastrophic (but silent) failure in setup_machine_fdt().
This commits addresses the issue as follows:
1. Call to fdt_nop_property is replaced with overwriting the rng-seed value with zeros.
2. Zeroed rng-seed gets special treatment and is not accepted as valid seed. Warning is emitted on zeroed value.
3. Kexec_file code is modified to delete the zeroed property if it can't fill it with valid seed.
4. Proper error handling is added for the case when DTB exceeds 2MB.
The change was tested in QEMU arm64 environment. To do so, kernel containing the change was built and included in buildroot initramfs. Subsequently, kernel was started in QEMU. Using kexec_file, new kernel was loaded and kexec reboot was issued. DTB size was noted in this step. After new kernel has booted, another kexec_file was issued. DTB size was confirmed not to change.
--- arch/arm64/kernel/machine_kexec_file.c | 5 +++++ drivers/of/fdt.c | 18 +++++++++++++++--- drivers/of/kexec.c | 12 +++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-)
arch/arm64/kernel/machine_kexec_file.c +5 -0
@@ -170,6 +170,11 @@ int load_other_segments(struct kimage *image,/* trim it */ fdt_pack (dtb); dtb_len =fdt_totalsize (dtb); +if (dtb_len >MAX_FDT_SIZE ) { +pr_err ("DTB exceeds the maximum size: 0x%lx > 0x%x" , dtb_len,MAX_FDT_SIZE ); + goto out_err; + } +pr_info ("DTB successfully created at 0x%lx (length 0x%lx)" , (unsigned long )dtb, dtb_len); kbuf.buffer = dtb; kbuf.bufsz = dtb_len; kbuf.mem =KEXEC_BUF_MEM_UNKNOWN ;
drivers/of/fdt.c +15 -3
@@ -1019,6 +1019,18 @@ int __init early_init_dt_scan_memory(void)return found_memory; } +static int check_randomness_nonzero (const uint8_t *rng_seed,int len) +{ +int i; + +for (i =0 ; i < len; i++) +if (rng_seed[i] !=0 ) +return true; + +pr_warn ("Provided rng-seed value is all zeros!" ); +return false; +} +int __init early_init_dt_scan_chosen (char *cmdline) {int l, node;
@@ -1039,11 +1051,11 @@ int __init early_init_dt_scan_chosen(char *cmdline)early_init_dt_check_for_elfcorehdr (node); rng_seed =of_get_flat_dt_prop (node,"rng-seed" , &l); -if (rng_seed && l >0 ) { +if (rng_seed && l >0 &&check_randomness_nonzero (rng_seed, l)) {add_bootloader_randomness (rng_seed, l); -/* try to clear seed so it won't be found. */ -fdt_nop_property (initial_boot_params, node,"rng-seed" ); +/* Zero out the rng-seed property */ +memset ((void *)rng_seed,0 , l);/* update CRC check value */ of_fdt_crc32 = crc32_be(~0, initial_boot_params,
drivers/of/kexec.c +11 -1
@@ -453,8 +453,18 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image, goto out;get_random_bytes (rng_seed,RNG_SEED_SIZE ); }else { -pr_notice ("RNG is not initialised: omitting \"%s\" property\n" , +pr_notice ("RNG is not initialised: deleting \"%s\" property\n" ,"rng-seed" ); +/* +* The rng-seed property may exist as zeroed stub. If so, +* remove it to not confuse the incoming kernel. +*/ + ret =fdt_delprop (fdt, chosen_node,"rng-seed" ); +if (ret == -FDT_ERR_NOTFOUND ) +/* It's fine */ + ret =0 ; +else if (ret) + goto out; } ret =fdt_setprop (fdt, chosen_node,"linux,booted-from-kexec" ,NULL ,0 );
Signature · 1 line
2.47.1
Rob Herring
maintainerQuoted 15 lines · Bartosz Szczepanek
As a part of platform boot, device tree is being read to extract randonmess bits. The 'rng-seed' property is used for that purpose. After reading the value, the field was overridden with NOP instead of being deleted or zeroed. The problem is that NOPed fields are later not reused, and kexec code appended this property every time DTB is prepared:
/* add rng-seed */
if (rng_is_initialized()) { void *rng_seed; ret = fdt_setprop_placeholder(dtb, off, FDT_PROP_RNG_SEED, RNG_SEED_SIZE, &rng_seed); if (ret) goto out; get_random_bytes(rng_seed, RNG_SEED_SIZE);
Quoted 8 lines
}
(source:arch/arm64/kernel/machine_kexec_file.c)Taken together, DTB grew at each kexec by 140 bytes ie. size of the newly added (and not overwritten) rng-seed property. ARM64 sets a hard limit on FDT size at 2MB, which means that after at most 14,979 kexecs DTB exceeded the limit causing catastrophic (but silent) failure in
setup_machine_fdt().
Just like 2MB should be enough for anyone, 14979 kexecs should be enough. ;)
Quoted 5 lines
This commits addresses the issue as follows:
1. Call tofdt_nop_propertyis replaced with overwriting the rng-seed value with zeros.
2. Zeroed rng-seed gets special treatment and is not accepted as valid seed. Warning is emitted on zeroed value.
How do you get a zeroed seed if you delete the property when zeroed? Sure, any random bootloader could do that, but that has nothing to do with kexec. And does it really hurt to add 0s to the random pool? A warning is fine. In any case, none of this is specific to DT seeds. It all belongs in the core if it is a problem.
Quoted 23 lines
3.
Kexec_filecode is modified to delete the zeroed property if it can't fill it with valid seed.
4. Proper error handling is added for the case when DTB exceeds 2MB.The change was tested in QEMU arm64 environment. To do so, kernel containing the change was built and included in buildroot initramfs. Subsequently, kernel was started in QEMU. Using
kexec_file, new kernel was loaded and kexec reboot was issued. DTB size was noted in this step. After new kernel has booted, anotherkexec_filewas issued. DTB size was confirmed not to change.Signed-off-by Bartosz Szczepanek <[email protected]>--- arch/arm64/kernel/machine_kexec_file.c | 5 +++++ drivers/of/fdt.c | 18 +++++++++++++++--- drivers/of/kexec.c | 12 +++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-)arch/arm64/kernel/machine_kexec_file.c +0
-0
arch/arm64/kernel/machine_kexec_file.c int load_other_segments(struct kimage *image, · line 174
Quoted 5 lines
arch/arm64/kernel/machine_kexec_file.c +2
-0@@ -170,6 +170,11 @@ int load_other_segments(struct kimage *image,/* trim it */ fdt_pack (dtb); dtb_len =fdt_totalsize (dtb); +if (dtb_len >MAX_FDT_SIZE ) { +pr_err ("DTB exceeds the maximum size: 0x%lx > 0x%x" , dtb_len,MAX_FDT_SIZE );
You can't check restrictions of the kexec'ed kernel in the current kernel. That restriction could be removed at any point and might not be a problem for the kexec'ed kernel.
Quoted 71 lines
arch/arm64/kernel/machine_kexec_file.c +3
-0+ goto out_err; + } +pr_info ("DTB successfully created at 0x%lx (length 0x%lx)" , (unsigned long )dtb, dtb_len); kbuf.buffer = dtb; kbuf.bufsz = dtb_len; kbuf.mem =KEXEC_BUF_MEM_UNKNOWN ;drivers/of/fdt.c +15
-3@@ -1019,6 +1019,18 @@ int __init early_init_dt_scan_memory(void)return found_memory; } +static int check_randomness_nonzero (const uint8_t *rng_seed,int len) +{ +int i; + +for (i =0 ; i < len; i++) +if (rng_seed[i] !=0 ) +return true; + +pr_warn ("Provided rng-seed value is all zeros!" ); +return false; +} +int __init early_init_dt_scan_chosen (char *cmdline) {int l, node;@@ -1039,11 +1051,11 @@ int __init early_init_dt_scan_chosen(char *cmdline)early_init_dt_check_for_elfcorehdr (node); rng_seed =of_get_flat_dt_prop (node,"rng-seed" , &l); -if (rng_seed && l >0 ) { +if (rng_seed && l >0 &&check_randomness_nonzero (rng_seed, l)) {add_bootloader_randomness (rng_seed, l); -/* try to clear seed so it won't be found. */ -fdt_nop_property (initial_boot_params, node,"rng-seed" ); +/* Zero out the rng-seed property */ +memset ((void *)rng_seed,0 , l);/* update CRC check value */ of_fdt_crc32 = crc32_be(~0, initial_boot_params,drivers/of/kexec.c +11
-2@@ -453,8 +453,18 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image, goto out;get_random_bytes (rng_seed,RNG_SEED_SIZE ); }else { -pr_notice ("RNG is not initialised: omitting \"%s\" property\n" , +pr_notice ("RNG is not initialised: deleting \"%s\" property\n" ,"rng-seed" ); +/* +* The rng-seed property may exist as zeroed stub. If so, +* remove it to not confuse the incoming kernel. +*/ + ret =fdt_delprop (fdt, chosen_node,"rng-seed" ); +if (ret == -FDT_ERR_NOTFOUND ) +/* It's fine */ + ret =0 ; +else if (ret) + goto out; } ret =fdt_setprop (fdt, chosen_node,"linux,booted-from-kexec" ,NULL ,0 ); --2.47.1
Quoted 23 lines
From: Rob Herring <[email protected]>
Sent: Monday, April 14, 2025 4:22 PM
To: Szczepanek, BartoszQuoted 18 lines · Bartosz Szczepanek
As a part of platform boot, device tree is being read to extract randonmess bits. The 'rng-seed' property is used for that purpose. After reading the value, the field was overridden with NOP instead of being deleted or zeroed. The problem is that NOPed fields are later not reused, and kexec code appended this property every time DTB is prepared:
/* add rng-seed */
if (rng_is_initialized()) {
void*rng_seed;
ret =fdt_setprop_placeholder(dtb, off, FDT_PROP_RNG_SEED, RNG_SEED_SIZE, &rng_seed);
if (ret)
goto out;
get_random_bytes(rng_seed, RNG_SEED_SIZE);
}
(source:arch/arm64/kernel/machine_kexec_file.c)
Quoted 7 lines
Quoted 5 lines
Taken together, DTB grew at each kexec by 140 bytes ie. size of the newly added (and not overwritten) rng-seed property. ARM64 sets a hard limit on FDT size at 2MB, which means that after at most 14,979 kexecs DTB exceeded the limit causing catastrophic (but silent) failure in
setup_machine_fdt().Just like 2MB should be enough for anyone, 14979 kexecs should be enough. ;)
I'm actually glad it wasn't more than that, I'm afraid if it were 149790 I would be still scratching my head about the occasional crashes out of nowhere (:
Quoted 8 lines
Quoted 7 lines
This commits addresses the issue as follows:
1. Call tofdt_nop_propertyis replaced with overwriting the rng-seed value with zeros.
2. Zeroed rng-seed gets special treatment and is not accepted as valid seed. Warning is emitted on zeroed value.
Quoted 5 lines
How do you get a zeroed seed if you delete the property when zeroed? Sure, any random bootloader could do that, but that has nothing to do with kexec. And does it really hurt to add 0s to the random pool? A warning is fine. In any case, none of this is specific to DT seeds. It all belongs in the core if it is a problem.
You're right that zeroed property should never get to early_init_dt_scan_chosen. If the outgoing kernel can't give us randomness, it removes the previously zeroed "rng-seed". So it's an extra check and we could perhaps do without it.
As for whether it hurts to add zeros to random pool, I actually wanted to avoid any doubts about security by handling that case explicitly. We can make it a mere warning if you like but I thought that illusion of randomness is worse than lack of randomness from a security standpoint. If we bail out on zeros, we are sure that under no circumstances we get misguided by "rng-seed" that is there but doesn't bring us actual entropy.
Quoted 19 lines
Quoted 19 lines
3.
Kexec_filecode is modified to delete the zeroed property if it can't fill it with valid seed.
4. Proper error handling is added for the case when DTB exceeds 2MB.The change was tested in QEMU arm64 environment. To do so, kernel containing the change was built and included in buildroot initramfs. Subsequently, kernel was started in QEMU. Using
kexec_file, new kernel was loaded and kexec reboot was issued. DTB size was noted in this step. After new kernel has booted, anotherkexec_filewas issued. DTB size was confirmed not to change.Signed-off-by Bartosz Szczepanek <[email protected]>---
arch/arm64/kernel/machine_kexec_file.c| 5 +++++ drivers/of/fdt.c | 18 +++++++++++++++--- drivers/of/kexec.c | 12 +++++++++++-3 files changed, 31 insertions(+), 4 deletions(-)
Quoted 14 lines
arch/arm64/kernel/machine_kexec_file.c int load_other_segments(struct kimage *image, · line 174
Quoted 10 lines
arch/arm64/kernel/machine_kexec_file.c +2
-0@@ -170,6 +170,11 @@ int load_other_segments(struct kimage *image, /* trim it */ fdt_pack(dtb); dtb_len = fdt_totalsize(dtb); + if (dtb_len > MAX_FDT_SIZE) { + pr_err("DTB exceeds the maximum size: 0x%lx > 0x%x", dtb_len, MAX_FDT_SIZE);You can't check restrictions of the kexec'ed kernel in the current kernel. That restriction could be removed at any point and might not be a problem for the kexec'ed kernel.
Fair point.
What would you say then for making it a warning? That could be a middle ground that would 1) allow us to kexec into more capable kernel without restriction, but also 2) help avoid lengthy investigations about early_init_dt_scan_chosen failures. Keep in mind that the function is so early that early consoles are not plugged in, so from reader perspective it's "Bye!" from the outgoing kernel followed by eternal silence.
Quoted 15 lines
Quoted 15 lines
arch/arm64/kernel/machine_kexec_file.c +3
-0+ goto out_err; + } + pr_info("DTB successfully created at 0x%lx (length 0x%lx)", (unsigned long)dtb, dtb_len); kbuf.buffer = dtb; kbuf.bufsz = dtb_len; kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;drivers/of/fdt.c +0
-0@@ -1019,6 +1019,18 @@ int __init early_init_dt_scan_memory(void) return found_memory; }
Quoted 1 line
drivers/of/fdt.c int __init early_init_dt_scan_memory(void) · line 1022
Quoted 1 line
drivers/of/fdt.c +1
-0+static int check_randomness_nonzero (const uint8_t *rng_seed,int len)
Self-review bit: it needs to be bool. I'll post a v2 once we have a common viewpoint on the matters above.
Kind regards,
Bartosz
Quoted 57 lines
Quoted 56 lines
drivers/of/fdt.c +14
-3+{ + int i; + + for (i = 0; i < len; i++) + if (rng_seed[i] != 0) + return true; + + pr_warn("Provided rng-seed value is all zeros!"); + return false; +} + int __init early_init_dt_scan_chosen(char *cmdline) { int l, node;@@ -1039,11 +1051,11 @@ int __init early_init_dt_scan_chosen(char *cmdline) early_init_dt_check_for_elfcorehdr(node); rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l); - if (rng_seed && l > 0) { + if (rng_seed && l > 0 && check_randomness_nonzero(rng_seed, l)) { add_bootloader_randomness(rng_seed, l); - /* try to clear seed so it won't be found. */ - fdt_nop_property(initial_boot_params, node, "rng-seed"); + /* Zero out the rng-seed property */ + memset((void *)rng_seed, 0, l); /* update CRC check value */ of_fdt_crc32 = crc32_be(~0, initial_boot_params,drivers/of/kexec.c +11
-2@@ -453,8 +453,18 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image, goto out; get_random_bytes(rng_seed, RNG_SEED_SIZE); } else { - pr_notice("RNG is not initialised: omitting \"%s\" property\n", + pr_notice("RNG is not initialised: deleting \"%s\" property\n", "rng-seed"); + /* + * The rng-seed property may exist as zeroed stub. If so, + * remove it to not confuse the incoming kernel. + */ + ret = fdt_delprop(fdt, chosen_node, "rng-seed"); + if (ret == -FDT_ERR_NOTFOUND) + /* It's fine */ + ret = 0; + else if (ret) + goto out; } ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0); --2.47.1
Rob Herring
maintainerQuoted 53 lines · Bartosz
Quoted 24 lines
From: Rob Herring <[email protected]>
Sent: Monday, April 14, 2025 4:22 PM
To: Szczepanek, BartoszQuoted 18 lines · Bartosz Szczepanek
As a part of platform boot, device tree is being read to extract randonmess bits. The 'rng-seed' property is used for that purpose. After reading the value, the field was overridden with NOP instead of being deleted or zeroed. The problem is that NOPed fields are later not reused, and kexec code appended this property every time DTB is prepared:
/* add rng-seed */
if (rng_is_initialized()) { void *rng_seed; ret = fdt_setprop_placeholder(dtb, off, FDT_PROP_RNG_SEED, RNG_SEED_SIZE, &rng_seed); if (ret) goto out; get_random_bytes(rng_seed, RNG_SEED_SIZE);}
(source:arch/arm64/kernel/machine_kexec_file.c)Quoted 7 lines
Quoted 5 lines
Taken together, DTB grew at each kexec by 140 bytes ie. size of the newly added (and not overwritten) rng-seed property. ARM64 sets a hard limit on FDT size at 2MB, which means that after at most 14,979 kexecs DTB exceeded the limit causing catastrophic (but silent) failure in
setup_machine_fdt().Just like 2MB should be enough for anyone, 14979 kexecs should be enough. ;)
I'm actually glad it wasn't more than that, I'm afraid if it were 149790 I would be still scratching my head about the occasional crashes out of nowhere (:
Quoted 8 lines
Quoted 7 lines
This commits addresses the issue as follows:
1. Call tofdt_nop_propertyis replaced with overwriting the rng-seed value with zeros.
2. Zeroed rng-seed gets special treatment and is not accepted as valid seed. Warning is emitted on zeroed value.Quoted 5 lines
How do you get a zeroed seed if you delete the property when zeroed? Sure, any random bootloader could do that, but that has nothing to do with kexec. And does it really hurt to add 0s to the random pool? A warning is fine. In any case, none of this is specific to DT seeds. It all belongs in the core if it is a problem.
You're right that zeroed property should never get to
early_init_dt_scan_chosen. If the outgoing kernel can't give us randomness, it removes the previously zeroed "rng-seed". So it's an extra check and we could perhaps do without it.
Quoted 6 lines
As for whether it hurts to add zeros to random pool, I actually wanted to avoid any doubts about security by handling that case explicitly. We can make it a mere warning if you like but I thought that illusion of randomness is worse than lack of randomness from a security standpoint. If we bail out on zeros, we are sure that under no circumstances we get misguided by "rng-seed" that is there but doesn't bring us actual entropy.
My point is your reasoning is true for all other callers of add_bootloader_randomness(), so shouldn't all of them be handled the same way? What guarantee do we have that UEFI implementation doesn't give us all zeros too? We know it can happen since we're also implementing the bootloader in this case, but that's not much better than not knowing. And there is the 'trust_bootloader' flag too. Do we want to warn about the bootloader seed if trust_bootloader==false? There's also the minor issue that in 1-in-a-gazillion times, the random value is all 0s. Does that matter? IDK. Really, you should ask the folks that implemented this feature in the first place.
Quoted 33 lines
Quoted 18 lines
Quoted 18 lines
3.
Kexec_filecode is modified to delete the zeroed property if it can't fill it with valid seed.
4. Proper error handling is added for the case when DTB exceeds 2MB.The change was tested in QEMU arm64 environment. To do so, kernel containing the change was built and included in buildroot initramfs. Subsequently, kernel was started in QEMU. Using
kexec_file, new kernel was loaded and kexec reboot was issued. DTB size was noted in this step. After new kernel has booted, anotherkexec_filewas issued. DTB size was confirmed not to change.Signed-off-by Bartosz Szczepanek <[email protected]>--- arch/arm64/kernel/machine_kexec_file.c | 5 +++++ drivers/of/fdt.c | 18 +++++++++++++++--- drivers/of/kexec.c | 12 +++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-)Quoted 14 lines
arch/arm64/kernel/machine_kexec_file.c int load_other_segments(struct kimage *image, · line 174
Quoted 10 lines
arch/arm64/kernel/machine_kexec_file.c +2
-0@@ -170,6 +170,11 @@ int load_other_segments(struct kimage *image,/* trim it */ fdt_pack (dtb); dtb_len =fdt_totalsize (dtb); +if (dtb_len >MAX_FDT_SIZE ) { +pr_err ("DTB exceeds the maximum size: 0x%lx > 0x%x" , dtb_len,MAX_FDT_SIZE );You can't check restrictions of the kexec'ed kernel in the current kernel. That restriction could be removed at any point and might not be a problem for the kexec'ed kernel.
Quoted 8 lines
Fair point.
What would you say then for making it a warning? That could be a middle ground that would 1) allow us to kexec into more capable kernel without restriction, but also 2) help avoid lengthy investigations about
early_init_dt_scan_chosenfailures. Keep in mind that the function is so early that early consoles are not plugged in, so from reader perspective it's "Bye!" from the outgoing kernel followed by eternal silence.
Well, I checked DTBs in the tree and the biggest is 178K, so we are a lot further off from this being a problem than I thought. So I guess a warning would be fine.
Rob