Fix: LTTng-modules ABI ioctl wrong direction

lttng-modules defines ioctl numbers (include/lttng/abi.h) using the
_IO*() macros. These macros are partly used to specify what type of
parameters that the ioctl will be expecting. It also specifies the
direction of the data flow.

    1. sending data from userspace to the kernel,
    2. sending data from the kernel to userspace,
    3. both.

According to the kernel's
Documentation/userspace-api/ioctl/ioctl-number.rst file here is the
meaning of the various macros:

 ====== == ============================================
 _IO    an ioctl with no parameters
 _IOW   an ioctl with write parameters (copy_from_user)
 _IOR   an ioctl with read parameters  (copy_to_user)
 _IOWR  an ioctl with both write and read parameters.
 ====== == ============================================

Some of our use of these macros are wrong. In some cases, we use _IOW()
when we should be using _IOR():

Here is a list of the ioctl numbers that should be _IOW() as they are
sending data to the kernel:

 #define LTTNG_KERNEL_SESSION_TRACK_PID      IOR(0xF6, 0x58, int32_t)
 #define LTTNG_KERNEL_SESSION_UNTRACK_PID    _IOR(0xF6, 0x59, int32_t)
 #define LTTNG_KERNEL_SESSION_SET_NAME       _IOR(0xF6, 0x5D, struct lttng_kernel_session_name)
 #define LTTNG_KERNEL_SESSION_SET_CREATION_TIME   _IOR(0xF6, 0x5E, struct lttng_kernel_session_creation_time)
 #define LTTNG_KERNEL_SESSION_LIST_TRACKER_IDS  _IOR(0xF6, 0xA0, struct lttng_kernel_tracker_args)
 #define LTTNG_KERNEL_SESSION_TRACK_ID          _IOR(0xF6, 0xA1, struct lttng_kernel_tracker_args)
 #define LTTNG_KERNEL_SESSION_UNTRACK_ID        _IOR(0xF6, 0xA2, struct lttng_kernel_tracker_args)

Fix this by changing the direction of the macros, but introduce "_OLD"
macros for backward compatibility.

User-space should gradually start interacting with the correct ioctl
direction. However, in order to preserve compatibility between newer
user-space tools and older lttng-modules, user-space should fall-back on
the "_OLD" ioctl directions if the new directions are not implemented by
an older lttng-modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: I70d1963464597e0e3b028a67739d4a13b96c1ea5

diff --git a/include/lttng/abi.h b/include/lttng/abi.h
index 5a5ad753800afb3c639f718d501092dbc4004055..40317e86c5f89e20811434ced24b9aed46aae564 100644 (file)
--- a/include/lttng/abi.h
+++ b/include/lttng/abi.h
@@ -368,9 +368,9 @@ struct lttng_kernel_abi_tracker_args {
 #define LTTNG_KERNEL_ABI_SESSION_START         _IO(0xF6, 0x56)
 #define LTTNG_KERNEL_ABI_SESSION_STOP          _IO(0xF6, 0x57)
 #define LTTNG_KERNEL_ABI_SESSION_TRACK_PID             \
-       _IOR(0xF6, 0x58, int32_t)
+       _IOW(0xF6, 0x58, int32_t)
 #define LTTNG_KERNEL_ABI_SESSION_UNTRACK_PID   \
-       _IOR(0xF6, 0x59, int32_t)
+       _IOW(0xF6, 0x59, int32_t)
 
 /*
  * ioctl 0x58 and 0x59 are duplicated here. It works, since _IOR vs _IO
@@ -383,9 +383,9 @@ struct lttng_kernel_abi_tracker_args {
 /* lttng/abi-old.h reserve 0x5A and 0x5B. */
 #define LTTNG_KERNEL_ABI_SESSION_STATEDUMP             _IO(0xF6, 0x5C)
 #define LTTNG_KERNEL_ABI_SESSION_SET_NAME              \
-       _IOR(0xF6, 0x5D, struct lttng_kernel_abi_session_name)
+       _IOW(0xF6, 0x5D, struct lttng_kernel_abi_session_name)
 #define LTTNG_KERNEL_ABI_SESSION_SET_CREATION_TIME             \
-       _IOR(0xF6, 0x5E, struct lttng_kernel_abi_session_creation_time)
+       _IOW(0xF6, 0x5E, struct lttng_kernel_abi_session_creation_time)
 
 /* Channel FD ioctl */
 /* lttng/abi-old.h reserve 0x60 and 0x61. */
@@ -415,11 +415,11 @@ struct lttng_kernel_abi_tracker_args {
 
 /* Session FD ioctl (continued) */
 #define LTTNG_KERNEL_ABI_SESSION_LIST_TRACKER_IDS      \
-       _IOR(0xF6, 0xA0, struct lttng_kernel_abi_tracker_args)
+       _IOW(0xF6, 0xA0, struct lttng_kernel_abi_tracker_args)
 #define LTTNG_KERNEL_ABI_SESSION_TRACK_ID              \
-       _IOR(0xF6, 0xA1, struct lttng_kernel_abi_tracker_args)
+       _IOW(0xF6, 0xA1, struct lttng_kernel_abi_tracker_args)
 #define LTTNG_KERNEL_ABI_SESSION_UNTRACK_ID            \
-       _IOR(0xF6, 0xA2, struct lttng_kernel_abi_tracker_args)
+       _IOW(0xF6, 0xA2, struct lttng_kernel_abi_tracker_args)
 
 /* Event notifier group file descriptor ioctl */
 #define LTTNG_KERNEL_ABI_EVENT_NOTIFIER_CREATE \
@@ -465,6 +465,25 @@ struct lttng_kernel_abi_tracker_args {
 /* returns the stream instance id (invariant for the stream) */
 #define LTTNG_KERNEL_ABI_RING_BUFFER_INSTANCE_ID               _IOR(0xF6, 0x28, uint64_t)
 
+/*
+ * Those ioctl numbers use the wrong direction, but are kept for ABI backward
+ * compatibility.
+ */
+#define LTTNG_KERNEL_ABI_OLD_SESSION_SET_NAME          \
+       _IOR(0xF6, 0x5D, struct lttng_kernel_abi_session_name)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_SET_CREATION_TIME \
+       _IOR(0xF6, 0x5E, struct lttng_kernel_abi_session_creation_time)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_TRACK_PID         \
+       _IOR(0xF6, 0x58, int32_t)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_UNTRACK_PID       \
+       _IOR(0xF6, 0x59, int32_t)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_LIST_TRACKER_IDS  \
+       _IOR(0xF6, 0xA0, struct lttng_kernel_abi_tracker_args)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_TRACK_ID          \
+       _IOR(0xF6, 0xA1, struct lttng_kernel_abi_tracker_args)
+#define LTTNG_KERNEL_ABI_OLD_SESSION_UNTRACK_ID                \
+       _IOR(0xF6, 0xA2, struct lttng_kernel_abi_tracker_args)
+
 #ifdef CONFIG_COMPAT
 /* returns the timestamp begin of the current sub-buffer */
 #define LTTNG_KERNEL_ABI_RING_BUFFER_COMPAT_GET_TIMESTAMP_BEGIN \

diff --git a/src/lttng-abi.c b/src/lttng-abi.c
index cc3a410cd9a5f62116b55015f2b038bbee0124e5..6829c239cc991c4a9d85016843b37b7d588dc8a9 100644 (file)
--- a/src/lttng-abi.c
+++ b/src/lttng-abi.c
@@ -793,6 +793,37 @@ long lttng_session_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
        struct lttng_kernel_abi_channel chan_param;
        struct lttng_kernel_abi_old_channel old_chan_param;
 
+       /*
+        * Handle backward compatibility. OLD commands have wrong
+        * directions, replace them by the correct direction.
+        */
+       switch (cmd) {
+       case LTTNG_KERNEL_ABI_OLD_SESSION_TRACK_PID:
+               cmd = LTTNG_KERNEL_ABI_SESSION_TRACK_PID;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_UNTRACK_PID:
+               cmd = LTTNG_KERNEL_ABI_SESSION_UNTRACK_PID;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_TRACK_ID:
+               cmd = LTTNG_KERNEL_ABI_SESSION_TRACK_ID;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_UNTRACK_ID:
+               cmd = LTTNG_KERNEL_ABI_SESSION_UNTRACK_ID;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_LIST_TRACKER_IDS:
+               cmd = LTTNG_KERNEL_ABI_SESSION_LIST_TRACKER_IDS;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_SET_NAME:
+               cmd = LTTNG_KERNEL_ABI_SESSION_SET_NAME;
+               break;
+       case LTTNG_KERNEL_ABI_OLD_SESSION_SET_CREATION_TIME:
+               cmd = LTTNG_KERNEL_ABI_SESSION_SET_CREATION_TIME;
+               break;
+       default:
+               /* Nothing to do. */
+               break;
+       }
+
        switch (cmd) {
        case LTTNG_KERNEL_ABI_OLD_CHANNEL:
        {