Maintain a list of supported IOCTL argument sizes and allow only those in
the list.
As an additional bonus, IOCTL handlers will be able to check whether the
caller actually set (using the argument size) the field vs. assigning it
to zero. Separate macro can be provided for that.
This will be easier for applications as well since there is no longer the
problem of setting the reserved fields zero, or at least it is a lesser
problem.
Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
Hi folks,
I've essentially addressed Mauro's comments on v2.
The code is only compile tested so far but the changes from the last
tested version are not that big. There's still some uncertainty though.
since v2:
- Rework is_valid_ioctl based on the comments
- Improved comments,
- Rename cmd as user_cmd, as this comes from the user
- Check whether there are alternative argument sizes before any
checks on IOCTL command if there is no exact match
- Use IOCSIZE_MASK macro instead of creating our own
- Add documentation for macros declaring IOCTLs
drivers/media/media-device.c | 98 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 92 insertions(+), 6 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 35e81f7..279d740 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -387,22 +387,65 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
/* Do acquire the graph mutex */
#define MEDIA_IOC_FL_GRAPH_MUTEX BIT(0)
-#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
+/**
+ * MEDIA_IOC_SZ_ARG - Declare a Media device IOCTL with alternative size and
+ * to_user/from_user callbacks
+ *
+ * @__cmd: The IOCTL command suffix (without "MEDIA_IOC_")
+ * @func: The handler function
+ * @fl: Flags from @enum media_ioc_flags
+ * @alt_sz: A 0-terminated list of alternative argument struct sizes.
+ * @from_user: Function to copy argument struct from the user to the kernel
+ * @to_user: Function to copy argument struct to the user from the kernel
+ */
+#define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user) \
[_IOC_NR(MEDIA_IOC_##__cmd)] = { \
.cmd = MEDIA_IOC_##__cmd, \
.fn = (long (*)(struct media_device *, void *))func, \
.flags = fl, \
+ .alt_arg_sizes = alt_sz, \
.arg_from_user = from_user, \
.arg_to_user = to_user, \
}
-#define MEDIA_IOC(__cmd, func, fl) \
- MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
+/**
+ * MEDIA_IOC_ARG - Declare a Media device IOCTL with to_user/from_user callbacks
+ *
+ * Just as MEDIA_IOC_SZ_ARG but without the alternative size list.
+ */
+#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
+ MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
+
+/**
+ * MEDIA_IOC_ARG - Declare a Media device IOCTL with alternative argument struct
+ * sizes
+ *
+ * Just as MEDIA_IOC_SZ_ARG but without the callbacks to copy the data from the
+ * user space and back to user space.
+ */
+#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz) \
+ MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, \
+ copy_arg_from_user, copy_arg_to_user)
+
+/**
+ * MEDIA_IOC_ARG - Declare a Media device IOCTL
+ *
+ * Just as MEDIA_IOC_SZ_ARG but without the alternative size list or the
+ * callbacks to copy the data from the user space and back to user space.
+ */
+#define MEDIA_IOC(__cmd, func, fl) \
+ MEDIA_IOC_ARG(__cmd, func, fl, \
+ copy_arg_from_user, copy_arg_to_user)
/* the table is indexed by _IOC_NR(cmd) */
struct media_ioctl_info {
unsigned int cmd;
unsigned short flags;
+ /*
+ * Sizes of the alternative arguments. If there are none, this
+ * pointer is NULL.
+ */
+ const unsigned short *alt_arg_sizes;
long (*fn)(struct media_device *dev, void *arg);
long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
@@ -416,6 +459,46 @@ static const struct media_ioctl_info ioctl_info[] = {
MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
};
+static inline long is_valid_ioctl(unsigned int user_cmd)
+{
+ const struct media_ioctl_info *info = ioctl_info;
+ const unsigned short *alt_arg_sizes;
+
+ if (_IOC_NR(user_cmd) >= ARRAY_SIZE(ioctl_info))
+ return -ENOIOCTLCMD;
+
+ info += _IOC_NR(user_cmd);
+
+ if (user_cmd == info->cmd)
+ return 0;
+
+ /*
+ * There was no exact match between the user-passed IOCTL command and
+ * the definition. Are there earlier revisions of the argument struct
+ * available?
+ */
+ if (!info->alt_arg_sizes)
+ return -ENOIOCTLCMD;
+
+ /*
+ * Variable size IOCTL argument support allows using either the latest
+ * revision of the IOCTL argument struct or an earlier version. Check
+ * that the size-independent portions of the IOCTL command match and
+ * that the size matches with one of the alternative sizes that
+ * represent earlier revisions of the argument struct.
+ */
+ if ((user_cmd & ~IOCSIZE_MASK) != (info->cmd & ~IOCSIZE_MASK)
+ || _IOC_SIZE(user_cmd) < _IOC_SIZE(info->cmd))
+ return -ENOIOCTLCMD;
+
+ for (alt_arg_sizes = info->alt_arg_sizes; *alt_arg_sizes;
+ alt_arg_sizes++)
+ if (_IOC_SIZE(user_cmd) == *alt_arg_sizes)
+ return 0;
+
+ return -ENOIOCTLCMD;
+}
+
static long media_device_ioctl(struct file *filp, unsigned int cmd,
unsigned long __arg)
{
@@ -426,9 +509,9 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
char __karg[256], *karg = __karg;
long ret;
- if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
- || ioctl_info[_IOC_NR(cmd)].cmd != cmd)
- return -ENOIOCTLCMD;
+ ret = is_valid_ioctl(cmd);
+ if (ret)
+ return ret;
info = &ioctl_info[_IOC_NR(cmd)];
@@ -444,6 +527,9 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
goto out_free;
}
+ /* Set the rest of the argument struct to zero */
+ memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
+
if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
mutex_lock(&dev->graph_mutex);
--
2.7.4
