Message ID | 20231019-kampfsport-metapher-e5211d7be247@brauner |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2010:b0:403:3b70:6f57 with SMTP id fe16csp275988vqb; Thu, 19 Oct 2023 03:09:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEh8UKKpr3wF3VHywWaF2/VPCBcQlKvuGZPg8HARrT1wOSpfPQyKFLfQXTUNSTwpLV0FRX0 X-Received: by 2002:a05:6a21:998f:b0:15d:53ad:22fe with SMTP id ve15-20020a056a21998f00b0015d53ad22femr2117381pzb.3.1697710161624; Thu, 19 Oct 2023 03:09:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697710161; cv=none; d=google.com; s=arc-20160816; b=tfAEdXy0LNna39Z8CN71OUQt6YB+ZaF35ObqGHSRfI1Vkn3IwRqrJ5n2mmucA+TVJT 1XkjDtTOHRtf1LNNqcZSMS018KnaIggkQHtg85s9I3Clm3Ei4hl98uNw7sszOK5tdc3j AsGcEnmXTwHZNsjBDIC0bvbHGSUJAEebUV3netBL8/ChWI/qJCofSDjgZvpcQ3d0Jws4 lH+kc8mwuVsNdq/mmfCmPo0Y0ywMeCqhCpgq6kTWERO/q3MMGr1XiilT37oCx3r5N8y9 u7I2035TRJlY7eZbk4rmxNDP7aT/+AP7LlEaKrQRYo1XGDfl9SPg8G5jwoLW7aG4AKaB urhA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=l3Ru+hcoIIN9vqhXhawfABhob/D9ouuZLvEP80mPyOA=; fh=qDZ+YnS9HJbCVOaeWFr2FnYZt7eE+vt5IJIDIlhBdxU=; b=CygEBqdGcZ38keAj0rl1LlXX3B0sN92hZNRXtu5JQUqkQc9Lj12wrONh+81+tUJsm4 Kzsh5Mdo3kbfZL71J+s+CzKAoMl4Ycfg0E2lhMLv58G8FzhBANkPJ84tqpUB33rPnHfX huLnjTRRLkCsJouYRMZEkfwxc4K+bhCUEHnvgt/lE8komEP6GNKioasSGwRoxQI92rkD mTN9KA1qOFvH0jmhHCkGGoJnU7Hd53FubN9oWJSHjGZlHh8alUIaGyA9NoTLZVoKl5zz s+AutpPOMIfnUmNn5hyVeOuvTnOhZ4mrR2mDfTbrS9NBc+bmDia5ICZ28ND8MncQGgrF nsgQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=bw11o5+I; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id u9-20020a056a00124900b0068a5752a049si6055907pfi.96.2023.10.19.03.09.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Oct 2023 03:09:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=bw11o5+I; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id B835B8289501; Thu, 19 Oct 2023 03:09:19 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345191AbjJSKJE (ORCPT <rfc822;zwp10758@gmail.com> + 24 others); Thu, 19 Oct 2023 06:09:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46404 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345115AbjJSKJE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 19 Oct 2023 06:09:04 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6AD7E119 for <linux-kernel@vger.kernel.org>; Thu, 19 Oct 2023 03:09:02 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CD16AC433C8; Thu, 19 Oct 2023 10:09:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1697710142; bh=96LKDRMrRpLCVaVRlRGnvTj5m9adaIRHeaHEEEyRJ08=; h=From:To:Cc:Subject:Date:From; b=bw11o5+I3+JgbuMc86sGALFuZS98+e+bGbUS8JrPwOxU3Q0ubZK2zl+9zdkOTHPbl wKz5mENFYv86mcaut0k3L6WbS7H4zVjxWymnyj6zy8qP3oFTR6oe8QtNOhSMve7JFG fFyza1W6Fcl52wiOaKODHGfHqazlRitBwfKjJ26tMkLCmptavmALEHLgKofwXXDnlX z0UA0YxLh7k4znCV7/pV5Oq6cUMsjh5X2px2wx9W9gSFC385JS9rl1dcYLlzoaIQIi 9F3szJfxBqg3TGb+C4XwjCgStcuppwk7GoJY++OY8nmbR8qWwMfCDNcLb/P8uzxyn7 T+d8nJh9yI68Q== From: Christian Brauner <brauner@kernel.org> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [GIT PULL] vfs fixes Date: Thu, 19 Oct 2023 12:07:08 +0200 Message-Id: <20231019-kampfsport-metapher-e5211d7be247@brauner> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1973; i=brauner@kernel.org; h=from:subject:message-id; bh=96LKDRMrRpLCVaVRlRGnvTj5m9adaIRHeaHEEEyRJ08=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMaQa/F/fd/vBj5BZmU0bllctWrzpRppozLYdMXIuN39+c513 8dqbaR2lLAxiXAyyYoosDu0m4XLLeSo2G2VqwMxhZQIZwsDFKQATeZfJ8N/lr4zrjkC5/WcirmQlHr ksNb204onEr+lb53Np3+cWFzjByPBol8mfCy/+Z2Zd/5vrzJmjxZd1a8O8uGfrzi7pOMh+bTY3AA== X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 19 Oct 2023 03:09:19 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780178130276929834 X-GMAIL-MSGID: 1780178130276929834 |
Series |
[GIT,PULL] vfs fixes
|
|
Pull-request
git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs tags/v6.6-rc7.vfs.fixesMessage
Christian Brauner
Oct. 19, 2023, 10:07 a.m. UTC
Hey Linus, /* Summary */ An openat() call from io_uring triggering an audit call can apparently cause the refcount of struct filename to be incremented from multiple threads concurrently during async execution, triggering a refcount underflow and hitting a BUG_ON(). That bug has been lurking around since at least v5.16 apparently. Switch to an atomic counter to fix that. The underflow check is downgraded from a BUG_ON() to a WARN_ON_ONCE() but we could easily remove that check altogether tbh and not waste an additional atomic. So if you feel that extra check isn't needed you could just remove in case you're pulling. /* Testing */ clang: Ubuntu clang version 15.0.7 gcc: (Ubuntu 12.2.0-3ubuntu1) 12.2.0 All patches are based on v6.6-rc6 and have been sitting in linux-next. No build failures or warnings were observed. /* Conflicts */ At the time of creating this PR no merge conflicts were reported from linux-next and no merge conflicts showed up doing a test-merge with current mainline. The following changes since commit 94f6f0550c625fab1f373bb86a6669b45e9748b3: Linux 6.6-rc5 (2023-10-08 13:49:43 -0700) are available in the Git repository at: git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs tags/v6.6-rc7.vfs.fixes for you to fetch changes up to 03adc61edad49e1bbecfb53f7ea5d78f398fe368: audit,io_uring: io_uring openat triggers audit reference count underflow (2023-10-13 18:34:46 +0200) Please consider pulling these changes from the signed v6.6-rc7.vfs.fixes tag. Thanks! Christian ---------------------------------------------------------------- v6.6-rc7.vfs.fixes ---------------------------------------------------------------- Dan Clash (1): audit,io_uring: io_uring openat triggers audit reference count underflow fs/namei.c | 9 +++++---- include/linux/fs.h | 2 +- kernel/auditsc.c | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-)
Comments
On Thu, 19 Oct 2023 at 03:09, Christian Brauner <brauner@kernel.org> wrote: > > An openat() call from io_uring triggering an audit call can apparently > cause the refcount of struct filename to be incremented from multiple > threads concurrently during async execution, triggering a refcount > underflow and hitting a BUG_ON(). That bug has been lurking around since > at least v5.16 apparently. Ouch. That filename ref by audit was always supposed to be thread-local in a "for this system call" kind of sense. But yes, looks like the io_uring stuff ended up making it no longer thread-local. That said, using atomics for reference counting is our default behavior and should be normal, so the patch isn't wrong, it's just annoying since getname/putname is very much in the critical path of filename handling. That said, the extra atomics are hopefully not really noticeable. Some people might want to use the non-refcounted version (ie we have getname/putname used by ksmbd too, for example), if they really care. It already exists, as __getname/__putname. But the normal open/stat/etc system call paths are obviously now going to hit those extra atomics. Not lovely, but I guess it's the best we can do. > Switch to an atomic counter to fix that. The underflow check is > downgraded from a BUG_ON() to a WARN_ON_ONCE() but we could easily > remove that check altogether tbh and not waste an additional atomic. So > if you feel that extra check isn't needed you could just remove in case > you're pulling. Well, the atomic *read* is cheap - the expensive part is the atomic_dec_and_test() (and the atomic_inc in the audit code. I'm not sure why you made it check just for zero in the WARN_ON_ONCE, rather than <= 0 as it used to, but that check is racy regardless, so it doesn't matter. It would miss two concurrent decrements coming in with a count of 1. We don't have the ternary test of atomic decrement results (positive, zero or negative), so it is what it is. Linus
The pull request you sent on Thu, 19 Oct 2023 12:07:08 +0200:
> git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs tags/v6.6-rc7.vfs.fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ea1cc20cd4ce55dd920a87a317c43da03ccea192
Thank you!
> Ouch. That filename ref by audit was always supposed to be > thread-local in a "for this system call" kind of sense. Yeah, I wasn't happy when that bug showed up. > That said, using atomics for reference counting is our default > behavior and should be normal, so the patch isn't wrong, it's just > annoying since getname/putname is very much in the critical path of > filename handling. Yeah. > That said, the extra atomics are hopefully not really noticeable. > > Some people might want to use the non-refcounted version (ie we have > getname/putname used by ksmbd too, for example), if they really care. > > It already exists, as __getname/__putname. > > But the normal open/stat/etc system call paths are obviously now going > to hit those extra atomics. Not lovely, but I guess it's the best we > can do. I didn't spend too much time on this issue because it's -rc7 and the straightforward seemed ok, if annoying. But if we really really really really cared we could probably do a deranged thing and massage both audit and io_uring to allows us to separate the regular system call getname from the io_uring getname. But I think that would be ugly and likely error prone.