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 E9334EB64DD for ; Wed, 5 Jul 2023 16:14:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230286AbjGEQOG (ORCPT ); Wed, 5 Jul 2023 12:14:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46532 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232086AbjGEQOF (ORCPT ); Wed, 5 Jul 2023 12:14:05 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3999A1700; Wed, 5 Jul 2023 09:14:04 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 8C022218EA; Wed, 5 Jul 2023 16:14:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1688573641; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ArN3dm9434Tcpervk5arIVNIxqqHdxPn08iNSUdER3s=; b=vUJGx30prcd9pjGwKtLTCS1kU+bGJcdsiVXayv0sVsAli0hJs3DphbYXGnQINP40uQwCgM ZhvUGi+mFF6F9+j2esjxXEgDNfKq4SBHVMa68J6rqCetob02iWnuu+wB6ZQe46bn08rjXA +ZVNUPajxlIi4DtdnYRn5JeDzanBiQE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1688573641; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ArN3dm9434Tcpervk5arIVNIxqqHdxPn08iNSUdER3s=; b=YrDkjE7MOQM2p+Kb0l3auCM5AiFEZ7rg+wXGy6hKCy5+M2PSh+3gNmHSWJcqFp9EntNST2 RQuGrits9C8XWAAw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 7BCD1134F3; Wed, 5 Jul 2023 16:14:01 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id SuwwHsmWpWQrQQAAMHmgww (envelope-from ); Wed, 05 Jul 2023 16:14:01 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id F238DA0707; Wed, 5 Jul 2023 18:14:00 +0200 (CEST) Date: Wed, 5 Jul 2023 18:14:00 +0200 From: Jan Kara To: Christian Brauner Cc: Jan Kara , linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org, Christoph Hellwig , Jens Axboe , Kees Cook , Ted Tso , syzkaller , Alexander Popov , Eric Biggers , linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, Dmitry Vyukov Subject: Re: [PATCH 6/6] fs: Make bind mounts work with bdev_allow_write_mounted=n Message-ID: <20230705161400.ihqbg7ftkdoa4ylf@quack3> References: <20230704122727.17096-1-jack@suse.cz> <20230704125702.23180-6-jack@suse.cz> <20230704-fasching-wertarbeit-7c6ffb01c83d@brauner> <20230705130033.ttv6rdywj5bnxlzx@quack3> <20230705-pumpwerk-vielversprechend-a4b1fd947b65@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230705-pumpwerk-vielversprechend-a4b1fd947b65@brauner> Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org On Wed 05-07-23 15:46:19, Christian Brauner wrote: > On Wed, Jul 05, 2023 at 03:00:33PM +0200, Jan Kara wrote: > > On Tue 04-07-23 15:59:41, Christian Brauner wrote: > > > On Tue, Jul 04, 2023 at 02:56:54PM +0200, Jan Kara wrote: > > > > When we don't allow opening of mounted block devices for writing, bind > > > > mounting is broken because the bind mount tries to open the block device > > > > > > Sorry, I'm going to be annoying now... > > > > > > Afaict, the analysis is misleading but I'm happy to be corrected ofc. > > > > I'm not sure what your objection exactly is. Probably I was imprecise in my > > changelog description. What gets broken by not allowing RW open of a > > mounted block device is: > > > > mount -t ext4 /dev/sda1 /mnt1 > > mount -t ext4 /dev/sda1 /mnt2 > > > > The second mount should create another mount of the superblock created by > > the first mount but before that is done, get_tree_bdev() tries to open the > > block device and fails when only patches 1 & 2 are applied. This patch > > fixes that. > > My objection is that this has nothing to do with mounts but with > superblocks. :) No mount need exist for this issue to appear. And I would > prefer if we keep superblock and mount separate as this leads to unclear > analysis and changelogs. > > > > > > Finding an existing superblock is independent of mounts. get_tree_bdev() > > > and mount_bdev() are really only interested in finding a matching > > > superblock independent of whether or not a mount for it already exists. > > > IOW, if you had two filesystem contexts for the same block device with > > > different mount options: > > > > > > T1 T2 > > > fd_fs = fsopen("ext4"); fd_fs = fsopen("ext4"); > > > fsconfig(fd_fs, FSCONFIG_SET_STRING, "source", "/dev/sda"); fsconfig(fd_fs, FSCONFIG_SET_STRING, "source", "/dev/sda"); > > > > > > // create superblock > > > fsconfig(fd_fs, FSCONFIG_CMD_CREATE, ...) > > > // finds superblock of T1 if opts are compatible > > > fsconfig(fd_fs, FSCONFIG_CMD_CREATE, ...) > > > > > > you should have the issue that you're describing. > > > > Correct, this will get broken when not allowing RW open for mounted block > > devices as well because the second fsconfig(fd_fs, FSCONFIG_CMD_CREATE, > > ...) will fail to open the block device in get_tree_bdev(). But again this > > patch should fix that. > > > > > But for neither of them does a mount already exist as the first mount > > > here would only be created when: > > > > > > T1 T2 > > > fsmount(fd_fs); fsmount(fd_fs); > > > > > > is called at which point the whole superblock issue is already settled. > > > Afterwards, both mounts of both T1 and T2 refer to the same superblock - > > > as long as the fs and the mount options support this ofc. > > > > I guess the confusion comes from me calling "mount" an operation as > > performed by the mount(8) command but which is in fact multiple operations > > with the new mount API. Anyway, is the motivation of this patch clearer > > now? > > I'm clear about what you're doing here. I would just like to not have > mounts brought into the changelog. Even before the new mount api what > you were describing was technically a superblock only issue. If someone > reads the changelog I want them to be able to clearly see that this is a > fix for superblocks, not mounts. > > Especially, since the code you touch really only has to to with > superblocks. > Let me - non ironically - return the question: Is my own request clearer > now? Yes, I understand now :). Thanks for explanation. I'll rephrase the changelog to speak about superblocks. Honza -- Jan Kara SUSE Labs, CR