diff mbox series

[RFC,v1,39/50] arm: kexec_file: Avoid temp buffer for RNG seed

Message ID 202003281643.02SGhMtr029198@sdf.org (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

George Spelvin Dec. 10, 2019, 3:45 p.m. UTC
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 <lkml@sdf.org>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Hsin-Yi Wang March 28, 2020, 7:04 p.m. UTC | #1
On Sun, Mar 29, 2020 at 12:43 AM George Spelvin <lkml@sdf.org> 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 <lkml@sdf.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org

Acked-by: Hsin-Yi Wang <hsinyi@chromium.org>

> ---
>  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
>
Mark Rutland March 30, 2020, 11:07 a.m. UTC | #2
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 <lkml@sdf.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  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 <mark.rutland@arm.com>

Mark.

>  	} else {
>  		pr_notice("RNG is not initialised: omitting \"%s\" property\n",
>  				FDT_PROP_RNG_SEED);
> -- 
> 2.26.0
>
Will Deacon March 30, 2020, 1:37 p.m. UTC | #3
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 <lkml@sdf.org>
> Cc: Hsin-Yi Wang <hsinyi@chromium.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/kernel/machine_kexec_file.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

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
diff mbox series

Patch

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);