From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4072DC7619A for ; Mon, 20 Mar 2023 22:59:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229523AbjCTW7j (ORCPT ); Mon, 20 Mar 2023 18:59:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37086 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229527AbjCTW7i (ORCPT ); Mon, 20 Mar 2023 18:59:38 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4F1B3C09; Mon, 20 Mar 2023 15:59:37 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7CA61618A6; Mon, 20 Mar 2023 22:59:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9DB9CC433EF; Mon, 20 Mar 2023 22:59:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679353176; bh=za2Gl7eeUWgx7zbEJwo9bzWn5tZ8aI+vrlqCpm0t8sE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UFKytX4Q33Rhin2D5ugs6L0tIMzUAcK7BqaWJ03fIiZCBn9ncz4aEg+YwYrfxBs6T TisZTe/l+EYQ6UkUpZUuA/+sy80QraO9frJn6omCWYlD/pwok5XKcDu1N8rEESWmXb Z0yORlSLS1A4Uu4iiPFkcJig26ct0AOFurx6+mnEzQAIjWBvL5pr3VGaJyPoePxGi0 Rn3BgbFgXqV9QZhRrBJ9P8xWpaE+MHvHG0VKXcXOBKv/jywYwu9tuKq4Jb5GBaoCqa gmAd9/7NflUZ2aeLcubhAmTyV6iZGJ8132teVju1vwWy3+W1cuP+I8ycXMtFDNoG5L j0Lbij3Mh5BGA== Date: Mon, 20 Mar 2023 15:59:34 -0700 From: Eric Biggers To: Linus Torvalds Cc: linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, Theodore Ts'o , Jaegeuk Kim Subject: Re: [GIT PULL] fscrypt fix for v6.3-rc4 Message-ID: <20230320225934.GB21979@sol.localdomain> References: <20230320205617.GA1434@sol.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: On Mon, Mar 20, 2023 at 03:16:48PM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 1:56 PM Eric Biggers wrote: > > > > fscrypt: check for NULL keyring in fscrypt_put_master_key_activeref() > > Side note: please just use WARN_ON_ONCE() for things like this, not WARN_ON. > > If it's triggerable, it should be triggered only once rather than > flood the logs and possibly cause a DoS. > > And if it's not triggerable, the "ONCE" doesn't matter. > > I note that fscypt in general seems to be *way* too happy with > WARN_ON() as some kind of debugging aid. > > It's not good in general (printf for debugging is wonderful, but > shouldn't be left in the sources to rot for all eternity), but it's > particularly not good in that form. > Yes, I agree that most of the WARN_ONs should be WARN_ON_ONCEs. I think I've been assuming that WARN_ON is significantly more lightweight than WARN_ON_ONCE. But that doesn't seem to be the case, especially since commit 19d436268dde ("debug: Add _ONCE() logic to report_bug()"). But besides that, I believe WARN* is generally being used appropriately in fs/crypto/. It's used when assumptions made by the code are violated, but where the hard crash of a BUG() is not necessary. I think this is a good thing to have, versus the alternative of doing nothing and making it much harder to track down bugs... Some particularly bad crypto bugs that we can easily WARN about, such as IVs being truncated, may not even be detectable by users otherwise. There are probably a few that should be removed, though. I'm also considering whether the refcounting-related ones should use CHECK_DATA_CORRUPTION, though that may run afoul of the "don't use BUG() unless absolutely needed" rule... - Eric