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
