Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230806230627.1394689-1-mjguzik@gmail.com> <87o7jidqlg.fsf@email.froward.int.ebiederm.org>
 <CAHk-=whk-8Pv5YXH4jNfyAf2xiQCGCUVyBWw71qJEafn4mT6vw@mail.gmail.com>
 <CAGudoHE5UDj0Y7fY=gicOq8Je=e1MX+5VWo04qoDRpHRG03fFg@mail.gmail.com>
 <CAHk-=wj+Uu+=iUZLc+MfOBKgRoyM56c0z0ustZKru0We9os63A@mail.gmail.com>
 <CAGudoHE=jJ+MKduj9-95Nk8_F=fkv2P+akftvFw1fVr46jm8ng@mail.gmail.com> <20230808-divers-verehren-02abcc37fe60@brauner>
In-Reply-To: <20230808-divers-verehren-02abcc37fe60@brauner>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 8 Aug 2023 10:48:19 -0700
Message-ID: <CAHk-=wgKpvn_u_9R72JbBaTw8gQnzhoER3hnR4WQpY8j96Gxcg@mail.gmail.com>
Subject: Re: [PATCH] fs: use __fput_sync in close(2)
To:     Christian Brauner <brauner@kernel.org>
Cc:     Mateusz Guzik <mjguzik@gmail.com>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, oleg@redhat.com,
        Matthew Wilcox <willy@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 8 Aug 2023 at 10:36, Christian Brauner <brauner@kernel.org> wrote:
>
> @Linus, you ok with the appended thing?

Yes.

I do think that the CHECK_DATA_CORRUPTION() case (used to be in
filp_close, now in filp_flush) is now very questionable since we'll
end up doing an "fput()" on it anyway.

But I think that's actually not a new thing - it was always in the
wrong place, and only caught the "filp_close()" cases. Which -
considering that it would only happen with people using 'fput()'
incorrectly - was always quite suspicious.

The actual "CHECK_DATA_CORRUPTION()" part of the check is new, but the
check itself predates not just the git tree, but the BK history too.
Google does find that we had it trigger back in 1998, apparently.

I think we should probably remove it entirely - and just depend on all
our modern use-after-free infrastructure.

Or we could move it into __fput() itself - the ordering wrt any
flushing is immaterial, because it's no different from using read or
write or whatever on a stale file descriptor - and at least get much
better coverage of the situation where it would happen.

But that is, I think, a completely separate issue - this is all just
made more obvious by the reorganization.

              Linus