Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Anand Jain <anand.jain@oracle.com>, Qu Wenruo <wqu@suse.com>,
	linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.cz>
Subject: Re: [PATCH] btrfs: reject unknown mount options early
Date: Fri, 29 Sep 2023 16:13:52 +0200	[thread overview]
Message-ID: <20230929141352.GD13697@twin.jikos.cz> (raw)
In-Reply-To: <8a89dbf2-c9dc-481a-8fdd-4aa26f86d59d@gmx.com>

On Thu, Sep 28, 2023 at 08:42:20AM +0930, Qu Wenruo wrote:
> On 2023/9/28 08:26, Anand Jain wrote:
> > On 27/09/2023 09:13, Qu Wenruo wrote:
> >> [BUG]
> >> The following script would allow invalid mount options to be specified
> >> (although such invalid options would just be ignored):
> >>
> >>   # mkfs.btrfs -f $dev
> >>   # mount $dev $mnt1        <<< Successful mount expected
> >>   # mount $dev $mnt2 -o junk    <<< Failed mount expected
> >>   # echo $?
> >>   0
> >>
> >> [CAUSE]
> >> For the 2nd mount, since the fs is already mounted, we won't go through
> >> open_ctree() thus no btrfs_parse_options(), but only through
> >> btrfs_parse_subvol_options().
> >>
> >> However we do not treat unrecognized options from valid but irrelevant
> >> options, thus those invalid options would just be ignored by
> >> btrfs_parse_subvol_options().
> >>
> >> [FIX]
> >> Add the handling for Opt_error to handle invalid options and error out,
> >> while still ignore other valid options inside
> >> btrfs_parse_subvol_options().
> >
> > As discussed, the purpose of my report was to determine whether we still
> > need to return success when the 'junk' option is passed in the second
> > mount. I don't recall precisely if this is intentional, perhaps to
> > allow future features to remain compatible with the KAPI when
> > backported to an older kernel version, or if it may not be relevant in
> > this kernel version.
> 
> This is not intentional, purely a bug.

Yeah it's a bug, we need to split the mount option parsting due to
device and subvoulme to preprocess some things but any invalid option at
the first phase is also invalid in the second one so there's no reason
to skip checking.

      reply	other threads:[~2023-09-29 14:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-27  1:13 [PATCH] btrfs: reject unknown mount options early Qu Wenruo
2023-09-27 15:32 ` David Sterba
2023-09-27 22:56 ` Anand Jain
2023-09-27 23:12   ` Qu Wenruo
2023-09-29 14:13     ` David Sterba [this message]

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230929141352.GD13697@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=anand.jain@oracle.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.com \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox