Re: lsattr: incorrect size for ioctl result
From: "Theodore Ts'o" <tytso@mit.edu>
Date: 2021-06-28 16:33:10
On Fri, Jun 25, 2021 at 04:01:27AM -0500, Rob Landley wrote:
quoted hunk ↗ jump to hunk
quoted
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