Skip to contentSkip to conversation

ioctl BLKGETSIZE64 fix

PATCH In review Coywolf Qi Hunt 3 participants 3 messages Dec 15, 2005 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ioctl BLKGETSIZE64 fix

Hello,

Two years ago, "[PATCH] use size_t for the broken ioctl numbers" brought in the problem.
<http://lkml.org/lkml/2003/9/7/14> (also FYI: <https://lwn.net/Articles/48360/>)

The patch below fixes the bug on BLKGETSIZE64. typeof(size_t) == 32, but we expect 64. The choice of size_t' is also a mistake. We should have taken int'. This also affects userland linux-kernel-headers.

Or am I missing something? Thanks.

Coywolf

---

include/linux/fs.h +1 -1
diff -pruN 2.6.15-rc5-mm3/include/linux/fs.h 2.6.15-rc5-mm3~BLKGETSIZE64-fix/include/linux/fs.h
@@ -197,7 +197,7 @@ extern int dir_notify_enable;
 /* A jump here: 108-111 have been used for various private purposes. */
 #define BLKBSZGET  _IOR(0x12,112,size_t)
 #define BLKBSZSET  _IOW(0x12,113,size_t)
-#define BLKGETSIZE64 _IOR(0x12,114,size_t)	/* return device size in bytes (u64 *arg) */
+#define BLKGETSIZE64 _IOR(0x12,114,u64)	/* return device size in bytes (u64 *arg) */
 #define BLKSTARTTRACE _IOWR(0x12,115,struct blk_user_trace_setup)
 #define BLKSTOPTRACE _IO(0x12,116)
 
#1 lore raw
Quoted 8 lines · Coywolf Qi Hunt

Two years ago, "[PATCH] use size_t for the broken ioctl numbers" brought in the problem.
<http://lkml.org/lkml/2003/9/7/14> (also FYI: <https://lwn.net/Articles/48360/>)

The patch below fixes the bug on BLKGETSIZE64. typeof(size_t) == 32, but we expect 64. The choice of size_t' is also a mistake. We should have taken int'. This also affects userland linux-kernel-headers.

Or am I missing something? Thanks.

Did you test this change? I don't think you understood what the original problem was.

This is the original definition:
-#define BLKGETSIZE64 _IOR(0x12,114,sizeof(u64)) /* return device
size in bytes (u64 *arg) */

What the author *meant* was to use u64. What they *got* was size_t. Unfortunately, changing this to u64 breaks compatibility with userspace. So we put in some checking code to make sure that people wouldn't make this kind of mistake any more and converted all the bad users to size_t.

With Linux the size argument is *just* a convention. Some Unices do the copyin/copyout thing in the ioctl dispatcher; Linux has the ioctl handler do the copyin/copyout. So it does no harm to have the wrong size.

In summary: This change is wrong and causes ABI breakage for architectures which have sizeof(u64) != sizeof(size_t).

Quoted 16 lines

Coywolf

---
diff -pruN 2.6.15-rc5-mm3/include/linux/fs.h 2.6.15-rc5-mm3~BLKGETSIZE64-fix/include/linux/fs.h

include/linux/fs.h +1 -1
@@ -197,7 +197,7 @@ extern int dir_notify_enable;
 /* A jump here: 108-111 have been used for various private purposes. */
 #define BLKBSZGET  _IOR(0x12,112,size_t)
 #define BLKBSZSET  _IOW(0x12,113,size_t)
-#define BLKGETSIZE64 _IOR(0x12,114,size_t)	/* return device size in bytes (u64 *arg) */
+#define BLKGETSIZE64 _IOR(0x12,114,u64)	/* return device size in bytes (u64 *arg) */
 #define BLKSTARTTRACE _IOWR(0x12,115,struct blk_user_trace_setup)
 #define BLKSTOPTRACE _IO(0x12,116)
 
#2 lore raw
In reply to Coywolf Qi Hunt
Quoted 9 lines · Coywolf Qi Hunt

Two years ago, "[PATCH] use size_t for the broken ioctl numbers" brought in the problem.
<http://lkml.org/lkml/2003/9/7/14> (also FYI: <https://lwn.net/Articles/48360/>)

The patch below fixes the bug on BLKGETSIZE64. typeof(size_t) == 32, but we expect 64. The choice of size_t' is also a mistake. We should have taken int'. This also affects userland linux-kernel-headers.

Or am I missing something? Thanks.

You're missing the fact that we can't just change existing numbers. It just makes the ioctl not work.

A lot of the "size_t"'s got added because people had mis-understood the _IOR/W macros, and had done a "sizeof()" by hand, when the macro itself did the sizeof. Which caused the macro to make the ioctl number be based on "sizeof(sizeof(realsize))", which is the same as "sizeof(size_t)".

And because we MUST NOT change the ioctl number (regardless of whether the number shows the correct size or not), those got changed to use "size_t" to match historical - if incorrect - numbering.

The _IOR/W macros got fixed so that you now _cannot_ use anything but a real type, so the bug shouldn't happen again, but we're stuck with the old broken numbers.

The number is just a number, after all. We shouldn't be using the size encoding in the ioctl number, afaik (since for a _lot_ of them we can't depend on it anyway).

Linus