external API: fix ioctl command definitions

This changes the ioctl commands to follow the common
conventions, resolving one item from the TODO file:

* We use _IO for commands that use the argument as a literal
  and not dereference it as a pointer. KDBUS_CMD_MEMFD_SEAL_SET
  just passes an bool here, and KDBUS_CMD_MSG_RELEASE passes
  a pointer that is compared to other pointers but not accessed.

* _IOR is used for commands that only copy data from the kernel
  to user space, IOW is used for commands that only copy into
  the kernel, which is the majority here. _IOWR is used only
  for commands that both read and write on the pointer passed
  as the ioctl arg. Whether we read or write from indirect
  pointers passed in the structure is not relevant here.

* The size argument must be the data structure that the ioctl
  argument points to. Importantly, this is not a pointer type
  as e.g. in the old KDBUS_CMD_MSG_RECV definition. Using
  a pointer here breaks 32 bit compat mode because taking
  sizeof(__u64*) results in a different command number on 32 and
  64 bit user space.

An open question is what to do for commands with variable-length
arguments. Some subsystems in the kernel actually define variable-length
commands as

 #define KDBUS_CMD_MSG_SEND(len) _IOC(_IOC_WRITE,KDBUS_IOC_MAGIC,\
				 0x40,sizeof(struct kdbus_msg)+(len))

This has the advantage of saving one 'copy_from_user' for each
call, and actually reflecting the correct size in the command
number, but it can also confuse readers. The maximum size that
can be passed like this is 8192 (alpha, mips, sparc, powerpc)
or 16384 (all other architectures) bytes, which may also be a
limitation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

2 files changed, 16 insertions, 17 deletions

diff --git a/TODO b/TODO
index b0b26e58fe8..05034560522 100644
--- a/TODO
+++ b/TODO
@@ -1,5 +1,4 @@
 External API:
-  - _IOWR() vs. _IOR()/_IOW()?
   - rules for: which unknown items to ignore in userspace?
 
 Internal:
diff --git a/kdbus.h b/kdbus.h
index 3b7783e1b16..48a97e61d78 100644
--- a/kdbus.h
+++ b/kdbus.h
@@ -403,35 +403,35 @@ struct kdbus_cmd_monitor {
  */
 enum {
 	/* kdbus control node commands: require unset state */
-	KDBUS_CMD_BUS_MAKE =		_IOWR(KDBUS_IOC_MAGIC, 0x00, struct kdbus_cmd_bus_make),
-	KDBUS_CMD_NS_MAKE =		_IOWR(KDBUS_IOC_MAGIC, 0x10, struct kdbus_cmd_ns_make),
+	KDBUS_CMD_BUS_MAKE =		_IOW(KDBUS_IOC_MAGIC, 0x00, struct kdbus_cmd_bus_make),
+	KDBUS_CMD_NS_MAKE =		_IOR(KDBUS_IOC_MAGIC, 0x10, struct kdbus_cmd_ns_make),
 
 	/* kdbus ep node commands: require unset state */
-	KDBUS_CMD_EP_MAKE =		_IOWR(KDBUS_IOC_MAGIC, 0x20, struct kdbus_cmd_ep_make),
+	KDBUS_CMD_EP_MAKE =		_IOW(KDBUS_IOC_MAGIC, 0x20, struct kdbus_cmd_ep_make),
 	KDBUS_CMD_HELLO =		_IOWR(KDBUS_IOC_MAGIC, 0x30, struct kdbus_cmd_hello),
 
 	/* kdbus ep node commands: require connected state */
-	KDBUS_CMD_MSG_SEND =		_IOWR(KDBUS_IOC_MAGIC, 0x40, struct kdbus_msg),
-	KDBUS_CMD_MSG_RECV =		_IOWR(KDBUS_IOC_MAGIC, 0x41, __u64 *),
-	KDBUS_CMD_MSG_RELEASE =		_IOWR(KDBUS_IOC_MAGIC, 0x42, __u64 *),
+	KDBUS_CMD_MSG_SEND =		_IOW(KDBUS_IOC_MAGIC, 0x40, struct kdbus_msg),
+	KDBUS_CMD_MSG_RECV =		_IOR(KDBUS_IOC_MAGIC, 0x41, __u64 *),
+	KDBUS_CMD_MSG_RELEASE =		_IOW(KDBUS_IOC_MAGIC, 0x42, __u64 *),
 
 	KDBUS_CMD_NAME_ACQUIRE =	_IOWR(KDBUS_IOC_MAGIC, 0x50, struct kdbus_cmd_name),
-	KDBUS_CMD_NAME_RELEASE =	_IOWR(KDBUS_IOC_MAGIC, 0x51, struct kdbus_cmd_name),
+	KDBUS_CMD_NAME_RELEASE =	_IOW(KDBUS_IOC_MAGIC, 0x51, struct kdbus_cmd_name),
 	KDBUS_CMD_NAME_LIST =		_IOWR(KDBUS_IOC_MAGIC, 0x52, struct kdbus_cmd_names),
 	KDBUS_CMD_NAME_QUERY =		_IOWR(KDBUS_IOC_MAGIC, 0x53, struct kdbus_cmd_name_info),
 
-	KDBUS_CMD_MATCH_ADD =		_IOWR(KDBUS_IOC_MAGIC, 0x60, struct kdbus_cmd_match),
-	KDBUS_CMD_MATCH_REMOVE =	_IOWR(KDBUS_IOC_MAGIC, 0x61, struct kdbus_cmd_match),
-	KDBUS_CMD_MONITOR =		_IOWR(KDBUS_IOC_MAGIC, 0x62, struct kdbus_cmd_monitor),
+	KDBUS_CMD_MATCH_ADD =		_IOW(KDBUS_IOC_MAGIC, 0x60, struct kdbus_cmd_match),
+	KDBUS_CMD_MATCH_REMOVE =	_IOW(KDBUS_IOC_MAGIC, 0x61, struct kdbus_cmd_match),
+	KDBUS_CMD_MONITOR =		_IOW(KDBUS_IOC_MAGIC, 0x62, struct kdbus_cmd_monitor),
 
 	/* kdbus ep node commands: require ep owner state */
-	KDBUS_CMD_EP_POLICY_SET =	_IOWR(KDBUS_IOC_MAGIC, 0x70, struct kdbus_cmd_policy),
+	KDBUS_CMD_EP_POLICY_SET =	_IOW(KDBUS_IOC_MAGIC, 0x70, struct kdbus_cmd_policy),
 
 	/* kdbus memfd commands: */
-	KDBUS_CMD_MEMFD_NEW =		_IOWR(KDBUS_IOC_MAGIC, 0x80, int *),
-	KDBUS_CMD_MEMFD_SIZE_GET =	_IOWR(KDBUS_IOC_MAGIC, 0x81, __u64 *),
-	KDBUS_CMD_MEMFD_SIZE_SET =	_IOWR(KDBUS_IOC_MAGIC, 0x82, __u64 *),
-	KDBUS_CMD_MEMFD_SEAL_GET =	_IOWR(KDBUS_IOC_MAGIC, 0x83, int *),
-	KDBUS_CMD_MEMFD_SEAL_SET =	_IOWR(KDBUS_IOC_MAGIC, 0x84, int),
+	KDBUS_CMD_MEMFD_NEW =		_IOR(KDBUS_IOC_MAGIC, 0x80, int *),
+	KDBUS_CMD_MEMFD_SIZE_GET =	_IOR(KDBUS_IOC_MAGIC, 0x81, __u64 *),
+	KDBUS_CMD_MEMFD_SIZE_SET =	_IOW(KDBUS_IOC_MAGIC, 0x82, __u64 *),
+	KDBUS_CMD_MEMFD_SEAL_GET =	_IOR(KDBUS_IOC_MAGIC, 0x83, int *),
+	KDBUS_CMD_MEMFD_SEAL_SET =	_IO(KDBUS_IOC_MAGIC, 0x84),
 };
 #endif