Re: [PATCH] random: make try_to_generate_entropy() more robust

[Thomas Gleixner]

On Fri, 18 Oct 2019, Linus Torvalds wrote:
> On Fri, Oct 18, 2019 at 4:42 PM JÃrn Engel <joern@xxxxxxxxxxxxxxx> wrote:
> >
> > We can generate entropy on almost any CPU, even if it doesn't provide a
> > high-resolution timer for random_get_entropy().  As long as the CPU is
> > not idle, it changed the register file every few cycles.  As long as the
> > ALU isn't fully synchronized with the timer, the drift between the
> > register file and the timer is enough to generate entropy from.
> 
> >  static void entropy_timer(struct timer_list *t)
> >  {
> > +     struct pt_regs *regs = get_irq_regs();
> > +
> > +     /*
> > +      * Even if we don't have a high-resolution timer in our system,
> > +      * the register file itself is a high-resolution timer.  It
> > +      * isn't monotonic or particularly useful to read the current
> > +      * time.  But it changes with every retired instruction, which
> > +      * is enough to generate entropy from.
> > +      */
> > +     mix_pool_bytes(&input_pool, regs, sizeof(*regs));
> 
> Ok, so I still like this conceptually, but I'm not entirely sure that
> get_irq_regs() works reliably in a timer. It's done from softirq
> TIMER_SOFTIRQ context, so not necessarily _in_ an interrupt.
> 
> Now, admittedly this code doesn't really need "reliably". The odd
> occasional hickup would arguably just add more noise. And I think the
> code works fine. get_irq_regs() will return a pointer to the last
> interrupt or exception frame on the current CPU, and I guess it's all
> fine. But let's bring in Thomas, who was not only active in the
> randomness discussion, but might also have stronger opinions on this
> get_irq_regs() usage.
> 
> Thomas, opinions? Using the register state (while we're doing the
> whole entropy load with scheduling etc) looks like a good source of
> high-entropy data outside of just the TSC, so it does seem like a very
> valid model. But I want to run it past more people first, and Thomas
> is the obvious victim^Wchoice.

The idea is good, but as Ingo pointed out this needs very careful checking
of 'regs'. get_irq_regs() is really only valid from interrupt context up to
the point where the old irq regs (default NULL) are restored, i.e. after
irq_exit() from where softirqs are invoked.

One slightly related thing I was looking into is that the mixing of
interrupt entropy is always done from hard interrupt context. That has a
few issues:

    1) It's pretty visible in profiles for high frequency interrupt
       scenarios.

    2) The regs content can be pretty boring non-deterministic when the
       interrupt hits idle.

       Not an issue in the try_to_generate_entropy() case probably, but
       that still needs some careful investigation.

For #1 I was looking into a trivial storage model with a per cpu ring
buffer, where each entry contains the entropy data of one interrupt and let
some thread or whatever handle the mixing later.

That would allow to filter out 'constant' data (#) but it would also give
Joerns approach a way to get to some 'random' register content independent
of the context in which the timer softirq is running in.

Thanks,

	tglx