[PATCH gnupg v10] Disable CPU speculation-related misfeatures

[Guido Trentalancia]

On Tue, 08/07/2025 at 20.02 -0500, Jacob Bachmeyer wrote:
> On 7/8/25 14:38, Guido Trentalancia via Gnupg-devel wrote:
> > The following new v10 patch has been created to fix a missing
> > #ifdef
> > and header file check for the case of L1D cache flushing.
> > [...]
> 
> Two major issues, further explained inline below:
>     - You are installing the signal handler incorrectly; this will
> interfere with other possible uses of SIGBUS.
>     - Your message from configure when testing the option to request
> L1 cache flushes is misleading.
> Further, none of this actually *fixes* anything; these are
> *workarounds* for widespread hardware bugs.

They are best practice on fixing the security vulnerabilities mentioned
in the patch. They are real "fixes" because they avoid the potential
information disclosure, they do not work around anything, they prevent
a series of hardware bugs from causing serious damage to data
confidentiality.

> Also, have you actually tested this on a machine where the request
> for L1 cache flushes raises SIGBUS?
> > diff -pru a/common/init.c b/common/init.c
> > --- a/common/init.c	2025-05-25 15:43:45.871984100 +0200
> > +++ b/common/init.c	2025-07-08 21:29:23.071406450 +0200
> > [...]
> >  /* This function should be the first called after main.  */
> >  void
> >  early_system_init (void)
> >  {
> > +#if defined(__linux__) && defined(HAVE_SYS_PRCTL_H)
> > +
> > +/* Disable CPU speculative execution security vulnerabilities
> > + * causing data leaks: see the Linux kernel documentation
> > + * Documentation/userspace-api/spec_ctrl.rst
> > + *
> > + * - Speculative Store Bypass (CVE-2018-3639, always
> > + *   disabled)
> > + * - Indirect Branch Speculation (CVE-2017-5715, always
> > + *   disabled)
> > + * - Flush L1D Cache on context switch out of the task (it
> > + *   requires the "nosmt l1d_flush=on" kernel boot parameter)
> > + */
> > +#ifdef PR_SPEC_STORE_BYPASS
> > +  prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS,
> > PR_SPEC_FORCE_DISABLE, 0, 0);
> > +#endif
> > +
> > +#ifdef PR_SPEC_INDIRECT_BRANCH
> > +  prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH,
> > PR_SPEC_FORCE_DISABLE, 0, 0);
> > +#endif
> > +
> > +#if defined(ENABLE_L1D_CACHE_FLUSH) && defined(PR_SPEC_L1D_FLUSH)
> > +  if (signal(SIGBUS, sigbus_handler) == SIG_ERR)
> > +    {
> > +      log_info ("Warning: cannot catch the SIGBUS signal.\n");
> > +    }
> 
> You cannot use signal() here in a library because you must restore
> any previous signal handler immediately after the prctl() call. 
> Other code in the program might have its own reasons to catch SIGBUS,
> and this handler will interfere with that. You must use sigaction()
> here to obtain the old handler in a form that you can use to restore
> it after calling prctl().

I have created a new v11 patch which uses sigaction(), as it has some
advantages over signal().

> > +  if (prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH,
> > PR_SPEC_ENABLE, 0, 0) < 0)
> > +    {
> > +      log_info ("Warning: Level 1 Data Cache flushing requires the
> > \"l1d_flush=on\" boot parameter.\n");
> > +    }
> 
> This is where you need to restore the previous state of SIGBUS
> handling.
> > +#endif
> > +
> > +#endif /* __linux__ && HAVE_SYS_PRCTL_H */
> >  }
> >  
> >  
> > diff -pru a/configure.ac b/configure.ac
> > --- a/configure.ac	2025-07-06 18:01:54.128546282 +0200
> > +++ b/configure.ac	2025-07-08 21:32:32.674405293 +0200
> > @@ -244,6 +244,16 @@ AC_ARG_ENABLE(selinux-support,
> >  AC_MSG_RESULT($selinux_support)
> >  
> >  
> > +# Fix security vulnerability CVE-2020-0550 by enabling
> > +# Level 1 Data Cache flushing on context switch.
> > +AC_MSG_CHECKING([whether Level 1 Data Cache is flushed on context
> > switch])
> 
> This message does not accurately describe what is going on.  This
> should say "whether L1 data cache should be flushed on context
> switch" because it does *not* test whether the cache is *actually*
> flushed, but only if the option to *request* flushing the cache is
> set.

The new code tests whether the cache is flushed or not by checking the
value returned by prctl() and when the cache is not flushed it produces
a warning message, so the comment above doesn't make any sense.

> > +AC_ARG_ENABLE(l1d-cache-flushing,
> > +              AS_HELP_STRING([--enable-l1d-cache-flushing],
> > +                             [enable L1D cache flushing]),
> > +              l1d_cache_flushing=$enableval,
> > l1d_cache_flushing=no)
> > +AC_MSG_RESULT($l1d_cache_flushing)
> > +
> > +
> >  AC_MSG_CHECKING([whether to allocate extra secure memory])
> >  AC_ARG_ENABLE(large-secmem,
> >                AS_HELP_STRING([--enable-large-secmem],
> > [...]
> 
> -- Jacob

The new v11 patch follows in a separate message...

Guido