Messages in this thread | | | From | Kees Cook <> | Date | Tue, 24 Apr 2018 13:04:58 -0700 | Subject | Re: [PATCH 1/3] big key: get rid of stack array allocation |
| |
On Tue, Apr 24, 2018 at 12:58 PM, Serge E. Hallyn <serge@hallyn.com> wrote: > Quoting Tycho Andersen (tycho@tycho.ws): >> On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote: >> > Tycho Andersen wrote: >> > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) { >> > > > > + WARN(1, "big key algorithm changed?"); >> > >> > Please avoid using WARN() WARN_ON() etc. >> > syzbot would catch it and panic() due to panic_on_warn == 1. >> >> But it is really a programming bug in this case (and it seems better >> than BUG()...). Isn't this exactly the sort of case we want to catch? >> >> Tycho > > Right - is there a url to some discussion about this? Because not > using WARN when WARN should be used, because it troubles a bot, seems > the wrong solution. If this *is* what's been agreed upon, then > what is the new recommended thing to do here?
BUG() is basically supposed to never be used, as decreed by Linus. WARN() here is entirely correct: if we encounter a case where crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE is not true, we run the risk of stack memory corruption. If this is an EXPECTED failure case, then okay, drop the WARN() but we have to keep the -EINVAL.
-Kees
-- Kees Cook Pixel Security
| |