Commit 59b01513 authored by Eric W. Biederman's avatar Eric W. Biederman Committed by Dmitry Torokhov
Browse files

Input: serio - fix potential deadlock when unbinding drivers 



sysfs_remove_group() waits for sysfs attributes to be removed, therefore
we do not need to worry about driver-specific attributes being accessed
after driver has been detached from the device. In fact, attempts to take
serio->drv_mutex in attribute methods may lead to the following deadlock:

                                          sysfs_read_file()
                                            fill_read_buffer()
                                              sysfs_get_active_two()
                                                psmouse_attr_show_helper()
                                                  serio_pin_driver()
serio_disconnect_driver()
  mutex_lock(&serio->drv_mutex);
                                <-------->        mutex_lock(&serio_drv_mutex);
    psmouse_disconnect()
      sysfs_remove_group(... psmouse_attr_group);
        ....
        sysfs_deactivate();
          wait_for_completion();

Fix this by removing calls to serio_[un]pin_driver() and functions themselves
and using driver-private mutexes to serialize access to attribute's set()
methods that may change device state.

Signed-off-by: default avatarEric W. Biederman <ebiederm@xmission.com>
Signed-off-by: default avatarDmitry Torokhov <dtor@mail.ru>
parent abf2a117

drivers/input/keyboard/atkbd.c

+25 −34
Original line number Diff line number Diff line
@@ -225,8 +225,10 @@ struct atkbd { 


	struct delayed_work event_work;

	unsigned long event_jiffies;

	struct mutex event_mutex;

	unsigned long event_mask;



	/* Serializes reconnect(), attr->set() and event work */

	struct mutex mutex;

};



/*
@@ -577,7 +579,7 @@ static void atkbd_event_work(struct work_struct *work) 
{

	struct atkbd *atkbd = container_of(work, struct atkbd, event_work.work);



	mutex_lock(&atkbd->event_mutex);

	mutex_lock(&atkbd->mutex);



	if (!atkbd->enabled) {

		/*
@@ -596,7 +598,7 @@ static void atkbd_event_work(struct work_struct *work) 
			atkbd_set_repeat_rate(atkbd);

	}



	mutex_unlock(&atkbd->event_mutex);

	mutex_unlock(&atkbd->mutex);

}



/*
@@ -612,7 +614,7 @@ static void atkbd_schedule_event_work(struct atkbd *atkbd, int event_bit) 


	atkbd->event_jiffies = jiffies;

	set_bit(event_bit, &atkbd->event_mask);

	wmb();

	mb();

	schedule_delayed_work(&atkbd->event_work, delay);

}
@@ -849,12 +851,13 @@ static void atkbd_disconnect(struct serio *serio) 
{

	struct atkbd *atkbd = serio_get_drvdata(serio);



	sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group);



	atkbd_disable(atkbd);



	/* make sure we don't have a command in flight */

	cancel_delayed_work_sync(&atkbd->event_work);



	sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group);

	input_unregister_device(atkbd->dev);

	serio_close(serio);

	serio_set_drvdata(serio, NULL);
@@ -1087,7 +1090,7 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv) 
	atkbd->dev = dev;

	ps2_init(&atkbd->ps2dev, serio);

	INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);

	mutex_init(&atkbd->event_mutex);

	mutex_init(&atkbd->mutex);



	switch (serio->id.type) {
@@ -1160,19 +1163,23 @@ static int atkbd_reconnect(struct serio *serio) 
{

	struct atkbd *atkbd = serio_get_drvdata(serio);

	struct serio_driver *drv = serio->drv;

	int retval = -1;



	if (!atkbd || !drv) {

		printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");

		return -1;

	}



	mutex_lock(&atkbd->mutex);



	atkbd_disable(atkbd);



	if (atkbd->write) {

		if (atkbd_probe(atkbd))

			return -1;

			goto out;



		if (atkbd->set != atkbd_select_set(atkbd, atkbd->set, atkbd->extra))

			return -1;

			goto out;



		atkbd_activate(atkbd);
@@ -1190,8 +1197,11 @@ static int atkbd_reconnect(struct serio *serio) 
	}



	atkbd_enable(atkbd);

	retval = 0;



	return 0;

 out:

	mutex_unlock(&atkbd->mutex);

	return retval;

}



static struct serio_device_id atkbd_serio_ids[] = {
@@ -1235,47 +1245,28 @@ static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf, 
				ssize_t (*handler)(struct atkbd *, char *))

{

	struct serio *serio = to_serio_port(dev);

	int retval;



	retval = serio_pin_driver(serio);

	if (retval)

		return retval;



	if (serio->drv != &atkbd_drv) {

		retval = -ENODEV;

		goto out;

	}



	retval = handler((struct atkbd *)serio_get_drvdata(serio), buf);

	struct atkbd *atkbd = serio_get_drvdata(serio);



out:

	serio_unpin_driver(serio);

	return retval;

	return handler(atkbd, buf);

}



static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t count,

				ssize_t (*handler)(struct atkbd *, const char *, size_t))

{

	struct serio *serio = to_serio_port(dev);

	struct atkbd *atkbd;

	struct atkbd *atkbd = serio_get_drvdata(serio);

	int retval;



	retval = serio_pin_driver(serio);

	retval = mutex_lock_interruptible(&atkbd->mutex);

	if (retval)

		return retval;



	if (serio->drv != &atkbd_drv) {

		retval = -ENODEV;

		goto out;

	}



	atkbd = serio_get_drvdata(serio);

	atkbd_disable(atkbd);

	retval = handler(atkbd, buf, count);

	atkbd_enable(atkbd);



out:

	serio_unpin_driver(serio);

	mutex_unlock(&atkbd->mutex);



	return retval;

}

drivers/input/mouse/psmouse-base.c

+3 −29
Original line number Diff line number Diff line
@@ -1450,24 +1450,10 @@ ssize_t psmouse_attr_show_helper(struct device *dev, struct device_attribute *de 
	struct serio *serio = to_serio_port(dev);

	struct psmouse_attribute *attr = to_psmouse_attr(devattr);

	struct psmouse *psmouse;

	int retval;



	retval = serio_pin_driver(serio);

	if (retval)

		return retval;



	if (serio->drv != &psmouse_drv) {

		retval = -ENODEV;

		goto out;

	}



	psmouse = serio_get_drvdata(serio);



	retval = attr->show(psmouse, attr->data, buf);



out:

	serio_unpin_driver(serio);

	return retval;

	return attr->show(psmouse, attr->data, buf);

}



ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *devattr,
@@ -1478,18 +1464,9 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev 
	struct psmouse *psmouse, *parent = NULL;

	int retval;



	retval = serio_pin_driver(serio);

	if (retval)

		return retval;



	if (serio->drv != &psmouse_drv) {

		retval = -ENODEV;

		goto out_unpin;

	}



	retval = mutex_lock_interruptible(&psmouse_mutex);

	if (retval)

		goto out_unpin;

		goto out;



	psmouse = serio_get_drvdata(serio);
@@ -1519,8 +1496,7 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev 


 out_unlock:

	mutex_unlock(&psmouse_mutex);

 out_unpin:

	serio_unpin_driver(serio);

 out:

	return retval;

}
@@ -1582,9 +1558,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co 
		}



		mutex_unlock(&psmouse_mutex);

		serio_unpin_driver(serio);

		serio_unregister_child_port(serio);

		serio_pin_driver_uninterruptible(serio);

		mutex_lock(&psmouse_mutex);



		if (serio->drv != &psmouse_drv) {

include/linux/serio.h

+0 −19
Original line number Diff line number Diff line
@@ -136,25 +136,6 @@ static inline void serio_continue_rx(struct serio *serio) 
	spin_unlock_irq(&serio->lock);

}



/*

 * Use the following functions to pin serio's driver in process context

 */

static inline int serio_pin_driver(struct serio *serio)

{

	return mutex_lock_interruptible(&serio->drv_mutex);

}



static inline void serio_pin_driver_uninterruptible(struct serio *serio)

{

	mutex_lock(&serio->drv_mutex);

}



static inline void serio_unpin_driver(struct serio *serio)

{

	mutex_unlock(&serio->drv_mutex);

}





#endif



/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE