From 8c71721f7a868b575b05e24bc3a3dcc967e6d5d6 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 20 Apr 2021 11:05:19 -0400 Subject: [PATCH] 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 Change-Id: I70d1963464597e0e3b028a67739d4a13b96c1ea5 --- include/lttng/abi.h | 33 ++++++++++++++++++++++++++------- src/lttng-abi.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/include/lttng/abi.h b/include/lttng/abi.h index 5a5ad753..40317e86 100644 --- 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 cc3a410c..6829c239 100644 --- 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: { -- 2.34.1