Input: uinput - rework ABS validation

Rework the uinput ABS validation to check passed absinfo data immediately,
but do ABS initialization as last step in UI_DEV_CREATE. The behavior
observed by user-space is not changed, as ABS initialization was never
checked for errors.

With this in place, the order of device initialization and abs
configuration is no longer fixed. Userspace can initialize the device and
afterwards set absinfo just fine.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

drivers/input/misc/uinput.c

@@ -256,13 +256,22 @@ static void uinput_destroy_device(struct uinput_device *udev)
 static int uinput_create_device(struct uinput_device *udev)
 {
 	struct input_dev *dev = udev->dev;
-	int error;
+	int error, nslot;
 
 	if (udev->state != UIST_SETUP_COMPLETE) {
 		printk(KERN_DEBUG "%s: write device info first\n", UINPUT_NAME);
 		return -EINVAL;
 	}
 
+	if (test_bit(ABS_MT_SLOT, dev->absbit)) {
+		nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
+		error = input_mt_init_slots(dev, nslot, 0);
+		if (error)
+			goto fail1;
+	} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
+		input_set_events_per_packet(dev, 60);
+	}
+
 	if (udev->ff_effects_max) {
 		error = input_ff_create(dev, udev->ff_effects_max);
 		if (error)
@@ -308,51 +317,50 @@ static int uinput_open(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int uinput_validate_absbits(struct input_dev *dev)
+static int uinput_validate_absinfo(struct input_dev *dev, unsigned int code,
+				   const struct input_absinfo *abs)
 {
-	unsigned int cnt;
-	int nslot;
-
-	if (!test_bit(EV_ABS, dev->evbit))
-		return 0;
-
-	/*
-	 * Check if absmin/absmax/absfuzz/absflat are sane.
-	 */
-
-	for_each_set_bit(cnt, dev->absbit, ABS_CNT) {
 	int min, max;
 
-		min = input_abs_get_min(dev, cnt);
-		max = input_abs_get_max(dev, cnt);
+	min = abs->minimum;
+	max = abs->maximum;
 
 	if ((min != 0 || max != 0) && max <= min) {
 		printk(KERN_DEBUG
 		       "%s: invalid abs[%02x] min:%d max:%d\n",
-				UINPUT_NAME, cnt,
-				input_abs_get_min(dev, cnt),
-				input_abs_get_max(dev, cnt));
+		       UINPUT_NAME, code, min, max);
 		return -EINVAL;
 	}
 
-		if (input_abs_get_flat(dev, cnt) >
-		    input_abs_get_max(dev, cnt) - input_abs_get_min(dev, cnt)) {
+	if (abs->flat > max - min) {
 		printk(KERN_DEBUG
-				"%s: abs_flat #%02x out of range: %d "
-				"(min:%d/max:%d)\n",
-				UINPUT_NAME, cnt,
-				input_abs_get_flat(dev, cnt),
-				input_abs_get_min(dev, cnt),
-				input_abs_get_max(dev, cnt));
+		       "%s: abs_flat #%02x out of range: %d (min:%d/max:%d)\n",
+		       UINPUT_NAME, code, abs->flat, min, max);
 		return -EINVAL;
 	}
-	}
 
-	if (test_bit(ABS_MT_SLOT, dev->absbit)) {
-		nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
-		input_mt_init_slots(dev, nslot, 0);
-	} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
-		input_set_events_per_packet(dev, 60);
+	return 0;
+}
+
+static int uinput_validate_absbits(struct input_dev *dev)
+{
+	unsigned int cnt;
+	int error;
+
+	if (!test_bit(EV_ABS, dev->evbit))
+		return 0;
+
+	/*
+	 * Check if absmin/absmax/absfuzz/absflat are sane.
+	 */
+
+	for_each_set_bit(cnt, dev->absbit, ABS_CNT) {
+		if (!dev->absinfo)
+			return -EINVAL;
+
+		error = uinput_validate_absinfo(dev, cnt, &dev->absinfo[cnt]);
+		if (error)
+			return error;
 	}
 
 	return 0;
@@ -375,7 +383,6 @@ static int uinput_dev_setup(struct uinput_device *udev,
 {
 	struct uinput_setup setup;
 	struct input_dev *dev;
-	int retval;
 
 	if (udev->state == UIST_CREATED)
 		return -EINVAL;
@@ -395,10 +402,6 @@ static int uinput_dev_setup(struct uinput_device *udev,
 	if (!dev->name)
 		return -ENOMEM;
 
-	retval = uinput_validate_absbits(dev);
-	if (retval < 0)
-		return retval;
-
 	udev->state = UIST_SETUP_COMPLETE;
 	return 0;
 }
@@ -408,6 +411,7 @@ static int uinput_abs_setup(struct uinput_device *udev,
 {
 	struct uinput_abs_setup setup = {};
 	struct input_dev *dev;
+	int error;
 
 	if (size > sizeof(setup))
 		return -E2BIG;
@@ -423,19 +427,16 @@ static int uinput_abs_setup(struct uinput_device *udev,
 
 	dev = udev->dev;
 
+	error = uinput_validate_absinfo(dev, setup.code, &setup.absinfo);
+	if (error)
+		return error;
+
 	input_alloc_absinfo(dev);
 	if (!dev->absinfo)
 		return -ENOMEM;
 
 	set_bit(setup.code, dev->absbit);
 	dev->absinfo[setup.code] = setup.absinfo;
-
-	/*
-	 * We restore the state to UIST_NEW_DEVICE because the user has to call
-	 * UI_DEV_SETUP in the last place before UI_DEV_CREATE to check the
-	 * validity of the absbits.
-	 */
-	udev->state = UIST_NEW_DEVICE;
 	return 0;
 }
 

include/uapi/linux/uinput.h

@@ -72,13 +72,12 @@ struct uinput_setup {
 /**
  * UI_DEV_SETUP - Set device parameters for setup
  *
- * This ioctl sets parameters for the input device to be created. It must be
- * issued *before* calling UI_DEV_CREATE or it will fail. This ioctl supersedes
- * the old "struct uinput_user_dev" method, which wrote this data via write().
- * To actually set the absolute axes, you also need to call the ioctl
- * UI_ABS_SETUP *before* calling this ioctl.
+ * This ioctl sets parameters for the input device to be created.  It
+ * supersedes the old "struct uinput_user_dev" method, which wrote this data
+ * via write(). To actually set the absolute axes UI_ABS_SETUP should be
+ * used.
  *
- * This ioctl takes a "struct uinput_setup" object as argument. The fields of
+ * The ioctl takes a "struct uinput_setup" object as argument. The fields of
  * this object are as follows:
  *              id: See the description of "struct input_id". This field is
  *                  copied unchanged into the new device.
@@ -87,9 +86,9 @@ struct uinput_setup {
  *                  See below for a description of FF with uinput.
  *
  * This ioctl can be called multiple times and will overwrite previous values.
- * If this ioctl fails with -EINVAL, you're recommended to use the old
- * "uinput_user_dev" method via write() as fallback, in case you run on an old
- * kernel that does not support this ioctl.
+ * If this ioctl fails with -EINVAL, it is recommended to use the old
+ * "uinput_user_dev" method via write() as a fallback, in case you run on an
+ * old kernel that does not support this ioctl.
  *
  * This ioctl may fail with -EINVAL if it is not supported or if you passed
  * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the
@@ -109,12 +108,10 @@ struct uinput_abs_setup {
  * UI_ABS_SETUP - Set absolute axis information for the device to setup
  *
  * This ioctl sets one absolute axis information for the input device to be
- * created. It must be issued *before* calling UI_DEV_SETUP and UI_DEV_CREATE
- * for every absolute axis the device exports.
- * This ioctl supersedes the old "struct uinput_user_dev" method, which wrote
+ * created. It supersedes the old "struct uinput_user_dev" method, which wrote
  * part of this data and the content of UI_DEV_SETUP via write().
  *
- * This ioctl takes a "struct uinput_abs_setup" object as argument. The fields
+ * The ioctl takes a "struct uinput_abs_setup" object as argument. The fields
  * of this object are as follows:
  *            code: The corresponding input code associated with this axis
  *                  (ABS_X, ABS_Y, etc...)
@@ -124,9 +121,9 @@ struct uinput_abs_setup {
  *                  UI_SET_ABSBIT, this ioctl will enable it.
  *
  * This ioctl can be called multiple times and will overwrite previous values.
- * If this ioctl fails with -EINVAL, you're recommended to use the old
- * "uinput_user_dev" method via write() as fallback, in case you run on an old
- * kernel that does not support this ioctl.
+ * If this ioctl fails with -EINVAL, it is recommended to use the old
+ * "uinput_user_dev" method via write() as a fallback, in case you run on an
+ * old kernel that does not support this ioctl.
  *
  * This ioctl may fail with -EINVAL if it is not supported or if you passed
  * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the