After using get_random_bytes(), you want to wipe the buffer
afterward so the seed remains secret.
In this case, we can eliminate the temporary buffer entirely.
fdt_setprop_placeholder returns a pointer to the property value
buffer, allowing us to put the random data directy in there without
using a temporary buffer at all. Faster and less stack all in one.
Signed-off-by: George Spelvin <[email protected]>
Cc: Hsin-Yi Wang <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: [email protected]
---
arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 7b08bf9499b6b..69e25bb96e3fb 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -106,12 +106,12 @@ static int setup_dtb(struct kimage *image,
/* add rng-seed */
if (rng_is_initialized()) {
- u8 rng_seed[RNG_SEED_SIZE];
- get_random_bytes(rng_seed, RNG_SEED_SIZE);
- ret = fdt_setprop(dtb, off, FDT_PROP_RNG_SEED, rng_seed,
- RNG_SEED_SIZE);
+ 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);
} else {
pr_notice("RNG is not initialised: omitting \"%s\" property\n",
FDT_PROP_RNG_SEED);
--
2.26.0
On Sun, Mar 29, 2020 at 12:43 AM George Spelvin <[email protected]> wrote:
>
> After using get_random_bytes(), you want to wipe the buffer
> afterward so the seed remains secret.
>
> In this case, we can eliminate the temporary buffer entirely.
> fdt_setprop_placeholder returns a pointer to the property value
> buffer, allowing us to put the random data directy in there without
> using a temporary buffer at all. Faster and less stack all in one.
>
> Signed-off-by: George Spelvin <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: [email protected]
Acked-by: Hsin-Yi Wang <[email protected]>
> ---
> arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 7b08bf9499b6b..69e25bb96e3fb 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -106,12 +106,12 @@ static int setup_dtb(struct kimage *image,
>
> /* add rng-seed */
> if (rng_is_initialized()) {
> - u8 rng_seed[RNG_SEED_SIZE];
> - get_random_bytes(rng_seed, RNG_SEED_SIZE);
> - ret = fdt_setprop(dtb, off, FDT_PROP_RNG_SEED, rng_seed,
> - RNG_SEED_SIZE);
> + 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);
> } else {
> pr_notice("RNG is not initialised: omitting \"%s\" property\n",
> FDT_PROP_RNG_SEED);
> --
> 2.26.0
>
Hi George,
Nit: s/arm/arm64/ in the title
On Tue, Dec 10, 2019 at 10:45:27AM -0500, George Spelvin wrote:
> After using get_random_bytes(), you want to wipe the buffer
> afterward so the seed remains secret.
>
> In this case, we can eliminate the temporary buffer entirely.
> fdt_setprop_placeholder returns a pointer to the property value
> buffer, allowing us to put the random data directy in there without
> using a temporary buffer at all. Faster and less stack all in one.
>
> Signed-off-by: George Spelvin <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: [email protected]
> ---
> arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 7b08bf9499b6b..69e25bb96e3fb 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -106,12 +106,12 @@ static int setup_dtb(struct kimage *image,
>
> /* add rng-seed */
> if (rng_is_initialized()) {
> - u8 rng_seed[RNG_SEED_SIZE];
> - get_random_bytes(rng_seed, RNG_SEED_SIZE);
> - ret = fdt_setprop(dtb, off, FDT_PROP_RNG_SEED, rng_seed,
> - RNG_SEED_SIZE);
> + 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);
This looks sane to me, so FWIW:
Acked-by: Mark Rutland <[email protected]>
Mark.
> } else {
> pr_notice("RNG is not initialised: omitting \"%s\" property\n",
> FDT_PROP_RNG_SEED);
> --
> 2.26.0
>
On Tue, Dec 10, 2019 at 10:45:27AM -0500, George Spelvin wrote:
> After using get_random_bytes(), you want to wipe the buffer
> afterward so the seed remains secret.
>
> In this case, we can eliminate the temporary buffer entirely.
> fdt_setprop_placeholder returns a pointer to the property value
> buffer, allowing us to put the random data directy in there without
s/directy/directly/
> using a temporary buffer at all. Faster and less stack all in one.
>
> Signed-off-by: George Spelvin <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: [email protected]
> ---
> arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Acked-by: Will Deacon <[email protected]>
Please let me know if you'd like this queued via the arm64 tree, as it
appears to be independent of the rest of this series.
Will
After using get_random_bytes(), you want to wipe the buffer
afterward so the seed remains secret.
In this case, we can eliminate the temporary buffer entirely.
fdt_setprop_placeholder() returns a pointer to the property value
buffer, allowing us to put the random data directly in there without
using a temporary buffer at all. Faster and less stack all in one.
Signed-off-by: George Spelvin <[email protected]>
Acked-by: Will Deacon <[email protected]>
Cc: Hsin-Yi Wang <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: [email protected]
---
v2: Typos in commit message fixed.
Thank you, I'd be delighted if you'd apply it to the arm64 tree directly!
I can take it out of my patch series and off my plate.
Now that I'm looking at it some more, I want to change
fdt_setprop_placeholder to return an ERR_PTR.
Must. Stop. Scope. Creep.
arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 7b08bf9499b6b..69e25bb96e3fb 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -106,12 +106,12 @@ static int setup_dtb(struct kimage *image,
/* add rng-seed */
if (rng_is_initialized()) {
- u8 rng_seed[RNG_SEED_SIZE];
- get_random_bytes(rng_seed, RNG_SEED_SIZE);
- ret = fdt_setprop(dtb, off, FDT_PROP_RNG_SEED, rng_seed,
- RNG_SEED_SIZE);
+ 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);
} else {
pr_notice("RNG is not initialised: omitting \"%s\" property\n",
FDT_PROP_RNG_SEED);
--
2.26.0
On Mon, 30 Mar 2020 17:38:01 +0000, George Spelvin wrote:
> After using get_random_bytes(), you want to wipe the buffer
> afterward so the seed remains secret.
>
> In this case, we can eliminate the temporary buffer entirely.
> fdt_setprop_placeholder() returns a pointer to the property value
> buffer, allowing us to put the random data directly in there without
> using a temporary buffer at all. Faster and less stack all in one.
Applied to arm64 (for-next/misc), thanks!
[1/1] arm64: kexec_file: Avoid temp buffer for RNG seed
https://git.kernel.org/arm64/c/99ee28d99607
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev