Re: lsattr: incorrect size for ioctl result

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

On Fri, Jun 25, 2021 at 04:01:27AM -0500, Rob Landley wrote:
> > No. The above is a lie.
> 
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -203,8 +203,8 @@ struct fsxattr {
> 
>  #define        FS_IOC_GETFLAGS                 _IOR('f', 1, long)
>  #define        FS_IOC_SETFLAGS                 _IOW('f', 2, long)
> -#define        FS_IOC_GETVERSION               _IOR('v', 1, long)
> -#define        FS_IOC_SETVERSION               _IOW('v', 2, long)
> +#define        FS_IOC_GETVERSION               _IOR('v', 1, unsigned int)
> +#define        FS_IOC_SETVERSION               _IOW('v', 2, unsigned int)
>  #define FS_IOC_FIEMAP                  _IOWR('f', 11, struct fiemap)
>  #define FS_IOC32_GETFLAGS              _IOR('f', 1, int)
>  #define FS_IOC32_SETFLAGS              _IOW('f', 2, int)

The problem is that there are a large number of userspace programs
which are using _IOR('v', 1, long) (the codepoint for
FS_IOC_GETVERSION for decades), but are expecting the kernel to fill
in an int.

We could do something like this:

#define        FS_IOC_GETVERSION               _IOR('v', 1, int)
#define        FS_IOC_GETVERSION_OLD           _IOR('v', 1, long)

But the key is that we keep support for the codepoint of _IOR('v', 1,
long) essentially forever, or we will break userspace binary
compatibility, which is verboten.

We also need to be a bit careful when we make these sorts of changes
of #defines, so we don't break kernel code like this: 

long ext2_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
	/* These are just misnamed, they actually get/put from/to user an int */
	switch (cmd) {
	case EXT2_IOC32_GETFLAGS:
		cmd = EXT2_IOC_GETFLAGS;
		break;
	case EXT2_IOC32_SETFLAGS:
		cmd = EXT2_IOC_SETFLAGS;
		break;
	case EXT2_IOC32_GETVERSION:
		cmd = EXT2_IOC_GETVERSION;
		break;
	case EXT2_IOC32_SETVERSION:
		cmd = EXT2_IOC_SETVERSION;
		break;
	default:
		return -ENOIOCTLCMD;
	}
	return ext2_ioctl(file, cmd, (unsigned long) compat_ptr(arg));
}

(This is from 4.4's fs/ext2/ioct.c; the point is if we want to "fix"
the definition of *_IOC_GETFLAGS because of a pearl clutching fit that
even though the code point is _IOR('v', 1, long), we're reading and
writing an int, we need to be careful and check all of the kernel
codepaths that refer to IOC_{GET,SET}{FLAGS,VERSION}.

> Which raises the question "why is there an IOC32 version of this when it was
> never NOT 32 bit" and "does GETFLAGS have the same problem"? (Haven't looked...)

Probably because the people who added the IOC32 versions didn't
understand this at the time?  :-)

					- Ted