Re: [PATCH] btrfs: reject unknown mount options early

[Anand Jain <anand.jain@oracle.com>]

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.

Thanks, Anand


> 
> Reported-by: Anand Jain <anand.jain@oracle.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/super.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 5c6054f0552b..574fcff0822f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -911,6 +911,10 @@ static int btrfs_parse_subvol_options(const char *options, char **subvol_name,
>   
>   			*subvol_objectid = subvolid;
>   			break;
> +		case Opt_err:
> +			btrfs_err(NULL, "unrecognized mount option '%s'", p);
> +			error = -EINVAL;
> +			goto out;
>   		default:
>   			break;
>   		}