Re: [PATCH] btrfs: add missing check for nocow and compression inode flags

From: David Sterba <dsterba@suse.cz>
To: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
Cc: David Sterba <dsterba@suse.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] btrfs: add missing check for nocow and compression inode flags
Date: Fri, 10 Jul 2020 15:28:14 +0200	[thread overview]
Message-ID: <20200710132814.GA3703@twin.jikos.cz> (raw)
In-Reply-To: <SN4PR0401MB35981E3913C16CB69DA42DF29B650@SN4PR0401MB3598.namprd04.prod.outlook.com>

On Fri, Jul 10, 2020 at 10:17:55AM +0000, Johannes Thumshirn wrote:
> On 10/07/2020 12:06, David Sterba wrote:
> > +static int check_fsflags(unsigned int old_flags, unsigned int flags)
> >  {
> >  	if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
> >  		      FS_NOATIME_FL | FS_NODUMP_FL | \
> > @@ -174,9 +177,19 @@ static int check_fsflags(unsigned int flags)
> >  		      FS_NOCOW_FL))
> >  		return -EOPNOTSUPP;
> >  
> > +	/* COMPR and NOCOMP on new/old are valid */
> >  	if ((flags & FS_NOCOMP_FL) && (flags & FS_COMPR_FL))
> >  		return -EINVAL;
> >  
> > +	if ((flags & FS_COMPR_FL) && (flags & FS_NOCOW_FL))
> > +		return -EINVAL;
> > +
> > +	/* NOCOW and compression options are mutually exclusive */
> > +	if ((old_flags & FS_NOCOW_FL) && (flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
> > +		return -EINVAL;
> > +	if ((flags & FS_NOCOW_FL) && (old_flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
> > +		return -EINVAL;
> > +
> 
> 
> If we'd pass in fs_info to check_fsflags() we could also validate against mount options
> which are incompatible with inode flags. Like -o nodatacow and FS_COMPR_FL or 
> -o auth_key and FS_NOCOW_FL.

Same question was asked on IRC too, mount options are independent and
take lower precedence than the inode attributes. A scenario where user
wants to set a nodatacow attribute when the filesystem is mounted with
compress= is valid and can be quite common.