SECRM, UNRM, COMPR flags

SECRM, UNRM, COMPR flags

[Jan Kara]

Hi,

in ext4 we have these SECRM, UNRM, COMPR flags which users can set, they
can read them, but which actually don't do anything. This is actually
somewhat confusing - e.g. I've just got report about one tool which
apparently sets SECRM flag on a file in a hope that it is somehow safer.
Also this is a waste of flags.

I've checked other filesystems (xfs, btrfs) and they report EOPNOTSUPP if
these flags are not really supported. Should not we do the same in ext4? I
know there is a concern about breaking userspace but since other major
filesystems already behave this way I think there is a good chance tools
handle this reasonably... What do people thing?

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: SECRM, UNRM, COMPR flags

[Theodore Ts'o]

On Mon, Sep 26, 2016 at 11:11:49AM +0200, Jan Kara wrote:
> 
> in ext4 we have these SECRM, UNRM, COMPR flags which users can set, they
> can read them, but which actually don't do anything. This is actually
> somewhat confusing - e.g. I've just got report about one tool which
> apparently sets SECRM flag on a file in a hope that it is somehow safer.
> Also this is a waste of flags.

I agree it doesn't seem very likely we'll be using UNRM any time soon.
I can imagine using SECRM and COMPR, but in particular for COMPR it
will probably be in a different way (the package manager would install
a file that would be compressed in userspace, and then using a
*different* ioctl from IOC_SETFLAGS, the COMPR flag would be set and
that would make the file immutable and the decompression would be done
in userspace).

> I've checked other filesystems (xfs, btrfs) and they report EOPNOTSUPP if
> these flags are not really supported. Should not we do the same in ext4? I
> know there is a concern about breaking userspace but since other major
> filesystems already behave this way I think there is a good chance tools
> handle this reasonably... What do people thing?

What we've been doing for other flags that we don't set is that we
simply mask them off (see EXT4_FL_USER_MODIFIABLE) so attempt so set
them will be a no-op.

What I think would make sense is to simply remove UNRM, SECRM, and
UNRM from the USER_MODIFIABLE bitmask.  I also suspect it might be
useful to define a new ioctl which returns the USER_VISIBLE and
USER_MODIFIABLE bitmasks, so that tools can know how to expect (and
give warning or error messages as desired).

What do folks think?

						- Ted

Re: SECRM, UNRM, COMPR flags

[Jan Kara]

On Mon 26-09-16 11:06:21, Ted Tso wrote:
> On Mon, Sep 26, 2016 at 11:11:49AM +0200, Jan Kara wrote:
> > 
> > in ext4 we have these SECRM, UNRM, COMPR flags which users can set, they
> > can read them, but which actually don't do anything. This is actually
> > somewhat confusing - e.g. I've just got report about one tool which
> > apparently sets SECRM flag on a file in a hope that it is somehow safer.
> > Also this is a waste of flags.
> 
> I agree it doesn't seem very likely we'll be using UNRM any time soon.
> I can imagine using SECRM and COMPR, but in particular for COMPR it
> will probably be in a different way (the package manager would install
> a file that would be compressed in userspace, and then using a
> *different* ioctl from IOC_SETFLAGS, the COMPR flag would be set and
> that would make the file immutable and the decompression would be done
> in userspace).
> 
> > I've checked other filesystems (xfs, btrfs) and they report EOPNOTSUPP if
> > these flags are not really supported. Should not we do the same in ext4? I
> > know there is a concern about breaking userspace but since other major
> > filesystems already behave this way I think there is a good chance tools
> > handle this reasonably... What do people thing?
> 
> What we've been doing for other flags that we don't set is that we
> simply mask them off (see EXT4_FL_USER_MODIFIABLE) so attempt so set
> them will be a no-op.

Yeah. This is better than what we currently have but still the problem is
that application can be tricked into thinking it got some functionality
when it actually did not (when ioctl succeeds, apps usually don't recheck
whether the bit actually got set - especially when other filesystems return
error in such case).

What I'd like is: Remove UNRM, SECRM and COMPR from USER_MODIFIABLE
bitmask. Return -EOPNOTSUPP when (flags & ~USER_MODIFIABLE) != 0.
This way we flag possible issues early and also using the so far unused
flags for any functionality in future is safe (otherwise you cannot be sure
whether some apps just randomly don't leave unused bits set). Whether some
apps won't get broken by this is a question but I'd hope not since as I
said other filesystems already behave this way and get away with that...
Are people willing to try this out?

> What I think would make sense is to simply remove UNRM, SECRM, and
> UNRM from the USER_MODIFIABLE bitmask.  I also suspect it might be
> useful to define a new ioctl which returns the USER_VISIBLE and
> USER_MODIFIABLE bitmasks, so that tools can know how to expect (and
> give warning or error messages as desired).

Well the GET/SETFLAGS ioctl is used by several filesystems these days so
we'd better check with other filesystems whether they are able to support
this functionality. I think they should be and it could be useful for app
to know which info it is able to get/set so that it doesn't have to
research through trial and error. But this is IMO a separate issue to the
above ext4-specific problem.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: SECRM, UNRM, COMPR flags

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 4051 bytes --]

On Sep 27, 2016, at 2:35 AM, Jan Kara <jack@suse.cz> wrote:
> 
> On Mon 26-09-16 11:06:21, Ted Tso wrote:
>> On Mon, Sep 26, 2016 at 11:11:49AM +0200, Jan Kara wrote:
>>> 
>>> in ext4 we have these SECRM, UNRM, COMPR flags which users can set, they
>>> can read them, but which actually don't do anything. This is actually
>>> somewhat confusing - e.g. I've just got report about one tool which
>>> apparently sets SECRM flag on a file in a hope that it is somehow safer.
>>> Also this is a waste of flags.
>> 
>> I agree it doesn't seem very likely we'll be using UNRM any time soon.
>> I can imagine using SECRM and COMPR, but in particular for COMPR it
>> will probably be in a different way (the package manager would install
>> a file that would be compressed in userspace, and then using a
>> *different* ioctl from IOC_SETFLAGS, the COMPR flag would be set and
>> that would make the file immutable and the decompression would be done
>> in userspace).
>> 
>>> I've checked other filesystems (xfs, btrfs) and they report EOPNOTSUPP if
>>> these flags are not really supported. Should not we do the same in ext4? I
>>> know there is a concern about breaking userspace but since other major
>>> filesystems already behave this way I think there is a good chance tools
>>> handle this reasonably... What do people thing?
>> 
>> What we've been doing for other flags that we don't set is that we
>> simply mask them off (see EXT4_FL_USER_MODIFIABLE) so attempt so set
>> them will be a no-op.
> 
> Yeah. This is better than what we currently have but still the problem is
> that application can be tricked into thinking it got some functionality
> when it actually did not (when ioctl succeeds, apps usually don't recheck
> whether the bit actually got set - especially when other filesystems return
> error in such case).
> 
> What I'd like is: Remove UNRM, SECRM and COMPR from USER_MODIFIABLE
> bitmask. Return -EOPNOTSUPP when (flags & ~USER_MODIFIABLE) != 0.
> This way we flag possible issues early and also using the so far unused
> flags for any functionality in future is safe (otherwise you cannot be sure
> whether some apps just randomly don't leave unused bits set). Whether some
> apps won't get broken by this is a question but I'd hope not since as I
> said other filesystems already behave this way and get away with that...
> Are people willing to try this out?

I know in the past that the "NODUMP" flag wasn't used directly by ext*,
but it was used by the "dump" tool to skip backing up files.  It may be
that tar checks this as well, but not sure.

I could imagine that "UNRM" and "SECRM" could be checked by userspace
tools (LD_PRELOAD or some desktop garbage can?) in userspace to see if
the file should be put into .Trash or shredded at unlink time, but I
don't know if that was ever implemented.  It definitely was suggested
several times when people asked about implementing this in the kernel.

Cheers, Andreas

>> What I think would make sense is to simply remove UNRM, SECRM, and
>> UNRM from the USER_MODIFIABLE bitmask.  I also suspect it might be
>> useful to define a new ioctl which returns the USER_VISIBLE and
>> USER_MODIFIABLE bitmasks, so that tools can know how to expect (and
>> give warning or error messages as desired).
> 
> Well the GET/SETFLAGS ioctl is used by several filesystems these days so
> we'd better check with other filesystems whether they are able to support
> this functionality. I think they should be and it could be useful for app
> to know which info it is able to get/set so that it doesn't have to
> research through trial and error. But this is IMO a separate issue to the
> above ext4-specific problem.
> 
> 								Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: SECRM, UNRM, COMPR flags

[Theodore Ts'o]

On Tue, Sep 27, 2016 at 10:35:08AM +0200, Jan Kara wrote:
> 
> What I'd like is: Remove UNRM, SECRM and COMPR from USER_MODIFIABLE
> bitmask. Return -EOPNOTSUPP when (flags & ~USER_MODIFIABLE) != 0.
> This way we flag possible issues early and also using the so far unused
> flags for any functionality in future is safe (otherwise you cannot be sure
> whether some apps just randomly don't leave unused bits set). Whether some
> apps won't get broken by this is a question but I'd hope not since as I
> said other filesystems already behave this way and get away with that...
> Are people willing to try this out?

I'm uncomfortable returning an error but also making changes to the
flags.  If we return an error, we shouldn't be modifying the flags.

I'm happy adding a new ioctl which returns an error without making any
changes --- but that's something all file systems would have to
support.

I'm also happy having an ioctl which returns the current
USER_MODIFIABLE bitmap (which is per-file system, by the way).

> 
> > What I think would make sense is to simply remove UNRM, SECRM, and
> > UNRM from the USER_MODIFIABLE bitmask.  I also suspect it might be
> > useful to define a new ioctl which returns the USER_VISIBLE and
> > USER_MODIFIABLE bitmasks, so that tools can know how to expect (and
> > give warning or error messages as desired).
> 
> Well the GET/SETFLAGS ioctl is used by several filesystems these days so
> we'd better check with other filesystems whether they are able to support
> this functionality.

As mentioned about, the USER_MODIFIABLE bitmask is a per-file system
thing, so we can change it for ext4 without needing to consult with
the other file systems.  (It's EXT4_USER_MODIFIABLE_FL, IIRC).

But I don't think we can return an error but also make changes to the
flag fields, or make any changes to the actual behavior of the current
ioctl without consulting the other file systems.

That's why I think the better approach is to define a new ioctl that
returns the user modifialbe and user visible bitmasks, and teach
userspace to give warning messages (or error out entirely) if the user
tries to set a flag which the file system is going to ignore.

If the ioctl isn't defined for the other file systems, we'll fall back
to the current behavior --- or if you like we can follow up the
SETFLAGS with a GETFLAGS and then report a warning if the flags aren't
checked.

This to me is much more of an user interface issue for chattr than
anything else, so I thin kit's better to fix this in chattr.

	       	    	       	      	 - Ted