On 6/24/21 5:07 AM, Denys Vlasenko wrote:
On Thu, Jun 24, 2021 at 10:53 AM Rob Landley [off-list ref] wrote:
quoted
On 6/23/21 6:41 AM, Denys Vlasenko wrote:
quoted
unsigned long flag = 0, version = 0;
int fd;
struct stat sb;
...
if (FLAG(v)) {
if (ioctl(fd, FS_IOC_GETVERSION, (void*)&version) < 0) goto LABEL2;
xprintf("%-10lu ", version);
}
if (ext2_getflag(fd, &sb, &flag) < 0) perror_msg("reading flags '%s'", path);
...
The above only works on little-endian.
(The hint is in zeroing assignments at the beginning).
...
quoted
These ioctls return an int-sized result, not long.
(Check the kernel source to verify).
Looks to me like the kernel source is taking a long?
include/uapi/linux/fs.h:#define FS_IOC_GETVERSION _IOR('v', 1, long)
No. The above is a lie.
If so that's a kernel bug. That was out of current git.
quoted
Has since 2006: https://github.com/torvalds/linux/commit/36695673b012
There's a FS_IOC32_GETVERSION but that's not what we're using here?
fs/ext2/ioctl.c:
...
case EXT2_IOC_GETFLAGS:
flags = ei->i_flags & EXT2_FL_USER_VISIBLE;
return put_user(flags, (int __user *) arg);
...
case EXT2_IOC_GETVERSION:
return put_user(inode->i_generation, (int __user *) arg);
To verify, replace
- unsigned long flag = 0, version = 0;
+ unsigned long flag = -1L, version = -1L;
and see what x86-64 version of the lsattr -v shows.
Hmmm, btrfs is also using int __user *arg, xfs is using int... you're right.
That's a bug in the kernel headers.
Except int isn't right either, it's gotta be unsigned or else it's negative half
the time (which is not what lsattr -v shows). But fs/xfs/xfs_ioctl.c
fs/btrfs/ioctl.c fs/ext2/ioctl.c are all using signed "int *". Not that it
matters quite as much what they do internally, but I think the right patch is
probably:
--- 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)
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...)
David: opinion?
Rob