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
