Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8253552b authored by Mohan Srinivasan's avatar Mohan Srinivasan Committed by Amit Pundir
Browse files

ANDROID: input: keychord: Fix races in keychord_write. 



There are multiple bugs caused by threads racing in keychord_write.
1) Threads racing through this function can cause the same element to
be added to a linked list twice (multiple calls to
input_register_handler() for the same input_handler struct). And the
races can also cause an element in a linked list that doesn't exist
attempted to be removed (multiple calls to input_unregister_handler()
with the same input_handler struct).
2) The races can also cause duplicate kfree's of the keychords
struct.

Bug: 64133562
Bug: 63974334
Change-Id: I6329a4d58c665fab5d3e96ef96391e07b4941e80
Signed-off-by: default avatarMohan Srinivasan <srmohan@google.com>
parent 92fc7f9a

drivers/input/misc/keychord.c

+60 −1
Original line number Diff line number Diff line
@@ -60,6 +60,10 @@ struct keychord_device { 
	unsigned char		head;

	unsigned char		tail;

	__u16			buff[BUFFER_SIZE];

	/* Bit to serialize writes to this device */

#define KEYCHORD_BUSY			0x01

	unsigned long		flags;

	wait_queue_head_t	write_waitq;

};



static int check_keychord(struct keychord_device *kdev,
@@ -172,7 +176,6 @@ static int keychord_connect(struct input_handler *handler, 
		goto err_input_open_device;



	pr_info("keychord: using input dev %s for fevent\n", dev->name);



	return 0;



err_input_open_device:
@@ -224,6 +227,41 @@ static ssize_t keychord_read(struct file *file, char __user *buffer, 
	return count;

}



/*

 * serializes writes on a device. can use mutex_lock_interruptible()

 * for this particular use case as well - a matter of preference.

 */

static int

keychord_write_lock(struct keychord_device *kdev)

{

	int ret;

	unsigned long flags;



	spin_lock_irqsave(&kdev->lock, flags);

	while (kdev->flags & KEYCHORD_BUSY) {

		spin_unlock_irqrestore(&kdev->lock, flags);

		ret = wait_event_interruptible(kdev->write_waitq,

			       ((kdev->flags & KEYCHORD_BUSY) == 0));

		if (ret)

			return ret;

		spin_lock_irqsave(&kdev->lock, flags);

	}

	kdev->flags |= KEYCHORD_BUSY;

	spin_unlock_irqrestore(&kdev->lock, flags);

	return 0;

}



static void

keychord_write_unlock(struct keychord_device *kdev)

{

	unsigned long flags;



	spin_lock_irqsave(&kdev->lock, flags);

	kdev->flags &= ~KEYCHORD_BUSY;

	spin_unlock_irqrestore(&kdev->lock, flags);

	wake_up_interruptible(&kdev->write_waitq);

}



/*

 * keychord_write is used to configure the driver

 */
@@ -250,6 +288,22 @@ static ssize_t keychord_write(struct file *file, const char __user *buffer, 
		return -EFAULT;

	}



	/*

	 * Serialize writes to this device to prevent various races.

	 * 1) writers racing here could do duplicate input_unregister_handler()

	 *    calls, resulting in attempting to unlink a node from a list that

	 *    does not exist.

	 * 2) writers racing here could do duplicate input_register_handler() calls

	 *    below, resulting in a duplicate insertion of a node into the list.

	 * 3) a double kfree of keychords can occur (in the event that

	 *    input_register_handler() fails below.

	 */

	ret = keychord_write_lock(kdev);

	if (ret) {

		kfree(keychords);

		return ret;

	}



	/* unregister handler before changing configuration */

	if (kdev->registered) {

		input_unregister_handler(&kdev->input_handler);
@@ -318,15 +372,19 @@ static ssize_t keychord_write(struct file *file, const char __user *buffer, 
	if (ret) {

		kfree(keychords);

		kdev->keychords = 0;

		keychord_write_unlock(kdev);

		return ret;

	}

	kdev->registered = 1;



	keychord_write_unlock(kdev);



	return count;



err_unlock_return:

	spin_unlock_irqrestore(&kdev->lock, flags);

	kfree(keychords);

	keychord_write_unlock(kdev);

	return -EINVAL;

}
@@ -352,6 +410,7 @@ static int keychord_open(struct inode *inode, struct file *file) 


	spin_lock_init(&kdev->lock);

	init_waitqueue_head(&kdev->waitq);

	init_waitqueue_head(&kdev->write_waitq);



	kdev->input_handler.event = keychord_event;

	kdev->input_handler.connect = keychord_connect;