From nobody Sat Aug 28 22:29:44 2021 X-Original-To: freebsd-fs@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 1A4CE17A0492 for ; Sat, 28 Aug 2021 22:29:54 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Gxrqj5Y2Wz3tCg for ; Sat, 28 Aug 2021 22:29:53 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 17SMTiqa035030 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Sun, 29 Aug 2021 01:29:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 17SMTiqa035030 Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 17SMTiat035029; Sun, 29 Aug 2021 01:29:44 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 29 Aug 2021 01:29:44 +0300 From: Konstantin Belousov To: Alexander Lochmann Cc: freebsd-fs , Horst Schirmeier Subject: Re: Various unprotected accesses to buf and vnode Message-ID: References: <55f3661e-2173-793e-4834-bbcd79d3d99e@tu-dortmund.de> <380bdcc8-bede-2a64-8e5e-031552231d82@tu-dortmund.de> <46649402-d28a-6f81-f0a8-39180b681f4c@tu-dortmund.de> List-Id: Filesystems List-Archive: https://lists.freebsd.org/archives/freebsd-fs List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-fs@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46649402-d28a-6f81-f0a8-39180b681f4c@tu-dortmund.de> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.5 X-Spam-Checker-Version: SpamAssassin 3.4.5 (2021-03-20) on tom.home X-Rspamd-Queue-Id: 4Gxrqj5Y2Wz3tCg X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Sat, Aug 28, 2021 at 11:16:15PM +0200, Alexander Lochmann wrote: > > > On 28.08.21 22:50, Konstantin Belousov wrote: > > On Sat, Aug 28, 2021 at 08:53:19PM +0200, Alexander Lochmann wrote: > > > On 27.08.21 20:40, Konstantin Belousov wrote: > > > > > - Read of buf.b_blkno in cluster_write(): > > > > > According to the documentation b_lock is needed. > > > > > Is b_blkno maybe a read-only element of struct buf? > > > > No, b_blkno is not read-only, it is the disk block number for the block, > > > > as opposed to b_lbklno which is logical file block number. The buffer > > > > is instantiated with b_blkno == b_lblkno, and when the buffer is mapped > > > > to the real disk block, b_blkno is updated. > > > > > > > > Could you show me the backtrace of the situation where cluster_write() > > > > is called with unlocked buffer? > > > > > > > > > > > > If you don't mind, you can find them here: > > > https://thasos.cs.tu-dortmund.de/bugs/ctx-buf-b_blkno-r-cex.html > > I see some banner "Counterexamples", two buttons, and then a blank space. > > Both under Firefox and Chrome under FreeBSD. > > > > I went as far as to ask and see what happens on Chrome under Windows, it > > looks the same. > Oh, I'm sorry. > Pls click on "Show Member List", and then select an entry. For both cases, > there should be only one entry. > (JavaScript must be enabled for this to work.) Ok, I see some call sequences (?), but again all of them are ffs_write() (one is ext2_write) calling into cluster_write(). There the buffer lock is owned. Show me the specific call sequence where it is not. > > > > > (Pls ignore the line 'Hypothesis 1: ....'.) > > > > > - Write to vnode.v_bufobj.bo_object: > > > > > https://github.com/freebsd/freebsd-src/blob/main/sys/vm/vnode_pager.c#L291 > > > > > According to the documentation, '[...] the vnode lock which embeds the > > > > > bufobj' is needed. However, interlock is taken in line 276. > > > > > Is the interlock equivalent to the vnode lock? > > > > > (I assume 'the vnode lock' refers to vnode.v_lock.) > > > > vnode_pager_alloc() must be called with the vnode exclusively locked. > > > > This is asserted at the beginning of the function. > > > > > > > Yeah, I see that check: ASSERT_VOP_LOCKED(vp, "vnode_pager_alloc");. > > > However, our data says otherwise: According to our trace, the shared lock is > > > taken. > > > You may have a look at https://thasos.cs.tu-dortmund.de/bugs/ctx-vnode-v_bufobj.bo_object-w-cex.html. > > > 'EMBSAME(vnode.v_lock[r])' refers to the shared vnode lock. > > > A click on each lock description, e.g., EMBSAME(vnode.v_lock[r]), leads to > > > the code where it was taken. > > > (Pls ignore the line 'Hypothesis 1: ....'.) Ah, yes, the calls from lookup and open would be with the shared lock. Still, we lock the vnode interlock to avoid double-allocating the v_object object, so it is fine. Some mode of the vnode lock is required nonetheless, because otherwise we might miss reclaim which guarantees that v_object is freed. > > > > > > > > > > - Is buf.b_bufobj a read-only element? > > > > You should scope the question. > > > > > > > > While buffer is owned by a vnode, b_bufobj is constant. Since buffers are > > > > type-stable, they migrate between vnodes as cache finds it required to > > > > reclaim and reuse. There, b_bufobj is changed. > > > > > > > I'm referring to getdirtybuf(): msleep(&bp->b_xflags, > > > BO_LOCKPTR(bp->b_bufobj),PRIBIO | PDROP, "getbuf", 0); > > And? The msleep() just uses the address as the sleep chain id. > > The getdirtybuf() function fails if it really slept, thus dropping bo lock > > and allowing the buffer identity to change (Really not identity, because > > vnode is locked, but allowing the buffer to be written out and moved to > > clean queue).