From: Christian Brauner <firstname.lastname@example.org>
Cc: Christoph Hellwig <email@example.com>, Al Viro <firstname.lastname@example.org>,
Hillf Danton <email@example.com>,
Subject: Re: [PATCH] fs: unset MNT_WRITE_HOLD on failure
Date: Thu, 21 Apr 2022 15:47:25 +0200 [thread overview]
Message-ID: <20220421134725.fdxt2rff4b7csuks@wittgenstein> (raw)
On Wed, Apr 20, 2022 at 03:19:25PM +0200, Christian Brauner wrote:
> After mnt_hold_writers() has been called we will always have set MNT_WRITE_HOLD
> and consequently we always need to pair mnt_hold_writers() with
> mnt_unhold_writers(). After the recent cleanup in  where Al switched from a
> do-while to a for loop the cleanup currently fails to unset MNT_WRITE_HOLD for
> the first mount that was changed. Fix this and make sure that the first mount
> will be cleaned up and add some comments to make it more obvious.
> Reported-by: firstname.lastname@example.org
> Reported-by: email@example.com
> Fixes: e257039f0fc7 ("mount_setattr(): clean the control flow and calling conventions") 
> Link: https://firstname.lastname@example.org
> Link: https://email@example.com
> Cc: Hillf Danton <firstname.lastname@example.org>
> Cc: Christoph Hellwig <email@example.com>
> Cc: Al Viro <firstname.lastname@example.org>
> Signed-off-by: Christian Brauner (Microsoft) <email@example.com>
> This should fix the syzbot issue. This is only relevant for making a
> mount or mount tree read-only:
> 1. successul recursive read-only mount tree change:
> Cleanup loop isn't executed.
> 2. failed recursive read-only mount tree change:
> m will point to the mount we failed to handle. The cleanup loop will
> run until p == m and then terminate.
> 3. successful single read-only mount change:
> Cleanup loop won't be executed.
> 4. failed single read-only mount change:
> m will point to mnt and the cleanup loop will terminate if p == m.
> I don't think there's any other weird corner cases since we now that
> MNT_WRITE_HOLD can only have been set by us as it requires
> lock_mount_hash() which we hold. So unconditionally unsetting it is
> fine. But please make sure to take a close look at the changed loop.
Unless I hear objections I'll route this upstream before -rc4 to get
this fixed because it's pretty trivial to trigger this.
next prev parent reply other threads:[~2022-04-21 13:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-19 16:16 [syzbot] INFO: rcu detected stall in sys_lsetxattr syzbot
2022-04-20 12:27 ` Christian Brauner
2022-04-20 13:01 ` syzbot
2022-04-20 13:19 ` [PATCH] fs: unset MNT_WRITE_HOLD on failure Christian Brauner
2022-04-21 13:47 ` Christian Brauner [this message]
2022-04-21 14:43 ` Christoph Hellwig
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.