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

* checkpatch.pl bug? (was Re: [PATCH] random: Make CPU trust a boot parameter)
@ 2018-08-27 21:55 Kees Cook
  2018-08-28  5:12 ` Joe Perches
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2018-08-27 21:55 UTC (permalink / raw)
  To: Joe Perches; +Cc: LKML

On Mon, Aug 27, 2018 at 2:51 PM, Kees Cook <keescook@chromium.org> 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@chromium.org>
> ---
>  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

^ permalink raw reply	[flat|nested] 4+ messages in thread