Re: [PATCH] hwrng: core - Add WARN_ON for buggy read return values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon Sep 23, 2024 at 5:48 PM EEST, Greg KH wrote:
> On Mon, Sep 23, 2024 at 05:31:47PM +0300, Jarkko Sakkinen wrote:
> > On Mon Sep 23, 2024 at 12:26 PM EEST, Herbert Xu wrote:
> >  > > +
> > > > > +		err = rng->read(rng, buffer, size, wait);
> > > > > +		if (WARN_ON_ONCE(err > 0 && err > size))
> > > > 
> > > > Are you sure you want to use WARN_ON_ONCE here instead of
> > > > pr_warn_once()? I.e. is it worth of taking down the whole
> > > > kernel?
> > >
> > > Absolutely.  If this triggers it's a serious kernel bug and we
> > > should gather as much information as possible.  pr_warn_once is
> > > not the same thing as WARN_ON_ONCE in terms of what it prints.
> > 
> > Personally I allow the use of WARN only as the last resort.
> > 
> > If you need stack printout you can always use dump_stack().
> > 
> > >
> > > If people want to turn WARNs into BUGs, then they've only got
> > > themselves to blame when the kernel goes down.  On the other
> > > hand perhaps they *do* want this to panic and we should hand
> > > it to them.
> > 
> > Actually when you turn on "panic_on_warn" the user expectation is and
> > should be that the sites where WARN is used have been hand picked with
> > consideration so that panic happens for a reason.
> > 
> > This has also been denoted repeatedly by Greg:
> > 
> > https://lore.kernel.org/linux-cve-announce/2024061828-CVE-2024-36975-6719@gregkh/
> > 
> > I should check this somewhere but actually these days a wrongly chosen
> > WARN() might lead to CVE entry. That fix was by me but I never created
> > the CVE.
> > 
> > Greg, did we have something under Documentation/ that would fully
> > address the use of WARN?
>
> Please see:
> 	https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
> which describes that.  We should make it more explicit that any WARN()
> or WARN_ON() calls that can be hit by user interactions somehow, will
> end up getting a CVE id when we fix it up to not do so.

I bookmarked this thanks :-)

Herbert, I'll do comprehensive testing tmrw by adding some invariants to
tpm2_get_random(). I'd really love to reimplement it because the current
implementation frankly sucks (and it's by me) but yep, we nee to fix it
first and foremost.

>
> thanks,
>
> greg k-h


BR, Jarkko





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux