Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls

[Theodore Ts'o <tytso@mit.edu>]

On Thu, Nov 28, 2013 at 05:53:10PM -0700, Andreas Dilger wrote:
> 
> Note that there are already FS_IOC32_{GET,SET}{VERSION,FLAGS} ioctl
> definitions for  that date back to 2.6.19 or so that correctly use "int"
> for the size argument.  Those are unfortunately(?) under CONFIG_COMPAT
> instead of just being inline with the normal ioctl definitions, so I'm
> not sure if those are available on the systems in question.  CONFIG_COMPAT
> was the default for RHEL5 and SLES10 kernels, but the compat ioctl code
> was only in ext4 and not ext3 in RHEL5 at least.

This were introduced to support 32-bit userspace programs (where
sizeof(long) == sizeof(int) == 4) with a 64-bit kernel.  The intent
was not to "fix" the ioctl, so much as it was to enable 32-bit
programs.  The compat code is in ext3 as well, although it uses
EXT3_IOC32_[SG]ETFLAGS.

> I suspect those have already been around long enough for chattr/lsattr to
> start trying to use them first, and fall back to using the "broken" IOC
> numbers if they fail.

Nope, chattr/lsattr does not try using them first, because the intent
wasn't to "fix" the "broken" IOC numbers.  If you look in the sources,
e2fsprogs is only using EXT2_IOC_[SG]ETFLAGS.  (These ioctls were
originally only defined for ext2, and intended for use only for
ext[23].  They got adopted by other file systems, and then they get
moved into linux/fs..)

> > P.S.  If we were going to create a new ioctl, what I'd suggest is that
> > the new ioctl explicitly use a 64-bit type, instead of using "long" or
> > "int", to avoid the compat ioctl hair to allow 64-bit kernels to
> > support 32-bit userspace programs.
> 
> Unfortunately, I don't think it would be possible to just use:
> 
> #define FS_IOC_GETFLAGS_NEW		_IOR('f', 1, __u64)
> 
> since this would conflict with the existing "long" definition on 64-bit
> platforms that actually expect an "int" argument.  It definitely would
> be desirable to get a 64-bit attributes API, since we are very close to
> running out of space in the 32-bit flags definitions.

Sure, I was thinking about doing something like this instead:

#define FS_IOC_GETFLAGS_WIDE		_IOR('f', 32, __u64)
#define FS_IOC_SETFLAGS_WIDE		_IOR('f', 32, __u64)

And I agree that a good reason to do this is to get 64 bits worth of
attributes....

Cheers,

						- Ted