Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Message-ID: <23109a06-0f1e-4baf-973b-d0a3d208ea65@kernel.dk>
Date:   Mon, 9 Oct 2023 07:40:54 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [EXTERNAL] Re: audit: io_uring openat triggers audit reference
 count underflow in worker thread
Content-Language: en-US
To:     Dan Clash <Dan.Clash@microsoft.com>,
        Paul Moore <paul@paul-moore.com>
Cc:     "audit@vger.kernel.org" <audit@vger.kernel.org>,
        "io-uring@vger.kernel.org" <io-uring@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "brauner@kernel.org" <brauner@kernel.org>
References: <MW2PR2101MB1033FFF044A258F84AEAA584F1C9A@MW2PR2101MB1033.namprd21.prod.outlook.com>
 <ab758860-e51e-409c-8353-6205fbe515dc@kernel.dk>
 <e0307260-c438-41d9-97ec-563e9932a60e@kernel.dk>
 <CAHC9VhQ0z4wuH7R=KRcUTyZuRs7adYTiH5JjohJSz4d2-Jd9EQ@mail.gmail.com>
 <MW2PR2101MB1033E52DD9307F9EF15F1E85F1CEA@MW2PR2101MB1033.namprd21.prod.outlook.com>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <MW2PR2101MB1033E52DD9307F9EF15F1E85F1CEA@MW2PR2101MB1033.namprd21.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/8/23 8:38 PM, Dan Clash wrote:
> I retested with the following change as a sanity check:
> 
> -       BUG_ON(name->refcnt <= 0);
> +       BUG_ON(atomic_read(&name->refcnt) <= 0);
> 
> checkpatch.pl suggests using WARN_ON_ONCE rather than BUG.
> 
> devvm ~ $ ~/linux/scripts/checkpatch.pl --patch ~/io_uring_audit_hang_atomic.patch 
> WARNING: Do not crash the kernel unless it is absolutely unavoidable
>   --use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants
> #28: FILE: fs/namei.c:262:
> +       BUG_ON(atomic_read(&name->refcnt) <= 0);
> ...
> 
> refcount_t uses WARN_ON_ONCE.
> 
> I can think of three choices:
> 
> 1. Use atomic_t and remove the BUG line.
> 2. Use refcount_t and remove the BUG line. 
> 3. Use atomic_t and partially implement the warn behavior of refcount_t.
> 
> Choice 1 and 2 seem better than choice 3.

I'd probably just make it:

if (WARN_ON_ONCE(!atomic_read(name->refcnt)))
	return;

to make it a straightforward conversion.

-- 
Jens Axboe