checkpatch.pl bug? (was Re: [PATCH] random: Make CPU trust a boot parameter)

From: Kees Cook
Date: Mon Aug 27 2018 - 17:55:54 EST


On Mon, Aug 27, 2018 at 2:51 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> Instead of forcing a distro or other system builder to choose
> at build time whether the CPU is trusted for CRNG seeding via
> CONFIG_RANDOM_TRUST_CPU, provide a boot-time parameter for end users to
> control the choice. The CONFIG will set the default state instead.
>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
> drivers/char/Kconfig | 4 ++--
> drivers/char/random.c | 11 ++++++++---
> 3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9871e649ffef..64a3bf54b974 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3523,6 +3523,12 @@
> ramdisk_size= [RAM] Sizes of RAM disks in kilobytes
> See Documentation/blockdev/ramdisk.txt.
>
> + random.trust_cpu={on,off}
> + [KNL] Enable or disable trusting the use of the
> + CPU's random number generator (if available) to
> + fully seed the kernel's CRNG. Default is controlled
> + by CONFIG_RANDOM_TRUST_CPU.
> +
> ras=option[,option,...] [KNL] RAS-specific options
>
> cec_disable [X86]
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index ce277ee0a28a..40728491f37b 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -566,5 +566,5 @@ config RANDOM_TRUST_CPU
> that CPU manufacturer (perhaps with the insistence or mandate
> of a Nation State's intelligence or law enforcement agencies)
> has not installed a hidden back door to compromise the CPU's
> - random number generation facilities.
> -
> + random number generation facilities. This can also be configured
> + at boot with "random.trust_cpu=on/off".
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index bf5f99fc36f1..c75b6cdf0053 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -779,6 +779,13 @@ static struct crng_state **crng_node_pool __read_mostly;
>
> static void invalidate_batched_entropy(void);
>
> +static bool trust_cpu __ro_after_init = IS_ENABLED(CONFIG_RANDOM_TRUST_CPU);
> +static int __init parse_trust_cpu(char *arg)
> +{
> + return kstrtobool(arg, &trust_cpu);
> +}
> +early_param("random.trust_cpu", parse_trust_cpu);
> +
> static void crng_initialize(struct crng_state *crng)
> {
> int i;
> @@ -799,12 +806,10 @@ static void crng_initialize(struct crng_state *crng)
> }
> crng->state[i] ^= rv;
> }
> -#ifdef CONFIG_RANDOM_TRUST_CPU
> - if (arch_init) {
> + if (trust_cpu && arch_init) {

checkpatch.pl complains:

ERROR: space prohibited after that '&&' (ctx:WxW)
#79: FILE: drivers/char/random.c:809:
+ if (trust_cpu && arch_init) {
^

I can't figure out what is going on here. Using "||" doesn't trigger
the issue; it seems related to the earlier "&trust_cpu" use in the
patch, but I can't figure out what checkpatch was trying to do with
this...

-Kees

> crng_init = 2;
> pr_notice("random: crng done (trusting CPU's manufacturer)\n");
> }
> -#endif
> crng->init_time = jiffies - CRNG_RESEED_INTERVAL - 1;
> }
>
> --
> 2.17.1
>
>
> --
> Kees Cook
> Pixel Security



--
Kees Cook
Pixel Security