media: input: Convert timers to use timer_setup()

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to perform its own repeat management. However, there is no path back to the existing status variable, so add a generic one to the input structure and use that instead.

Cc Dmitry Torokhov <[email protected]>

Cc Mauro Carvalho Chehab <[email protected]>

Cc Hans Verkuil <[email protected]>

Cc Sakari Ailus <[email protected]>

Cc Geliang Tang <[email protected]>

Cc [email protected]

Cc [email protected]

Signed-off-by Kees Cook <[email protected]>

Acked-by Pali Rohár <[email protected]>

---
 drivers/input/input.c               | 12 ++++++------
 drivers/media/pci/ttpci/av7110.h    |  1 -
 drivers/media/pci/ttpci/av7110_ir.c | 16 ++++++++--------
 include/linux/input.h               |  2 ++
 4 files changed, 16 insertions(+), 15 deletions(-)

drivers/input/input.c +6 -6

index d268fdc23c64..497ad2dcb699 100644
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int code)
 {
 	if (test_bit(EV_REP, dev->evbit) &&
 	    dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
-	    dev->timer.data) {
+	    dev->timer.function) {
 		dev->repeat_key = code;
 		mod_timer(&dev->timer,
 			  jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));

@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
  * dev->event_lock here to avoid racing with input_event
  * which may cause keys get "stuck".
  */
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
 {
-	struct input_dev *dev = (void *) data;
+	struct input_dev *dev = from_timer(dev, t, timer);
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);

@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
 		device_initialize(&dev->dev);
 		mutex_init(&dev->mutex);
 		spin_lock_init(&dev->event_lock);
-		init_timer(&dev->timer);
+		timer_setup(&dev->timer, NULL, 0);
 		INIT_LIST_HEAD(&dev->h_list);
 		INIT_LIST_HEAD(&dev->node);
 

@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device *dev, void *res)
  */
 void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
 {
-	dev->timer.data = (unsigned long) dev;
-	dev->timer.function = input_repeat_key;
+	dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+	dev->timer_data = 0;
 	dev->rep[REP_DELAY] = delay;
 	dev->rep[REP_PERIOD] = period;
 }

drivers/media/pci/ttpci/av7110.h +0 -1

index 347827925c14..b98a4f3006df 100644
@@ -93,7 +93,6 @@ struct infrared {
 	u8			inversion;
 	u16			last_key;
 	u16			last_toggle;
-	u8			delay_timer_finished;
 };
 
 

drivers/media/pci/ttpci/av7110_ir.c +8 -8

index ca05198de2c2..a883caa6488c 100644
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
 	if (timer_pending(&ir->keyup_timer)) {
 		del_timer(&ir->keyup_timer);
 		if (ir->last_key != keycode || toggle != ir->last_toggle) {
-			ir->delay_timer_finished = 0;
+			ir->input_dev->timer_data = 0;
 			input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
 			input_event(ir->input_dev, EV_KEY, keycode, 1);
 			input_sync(ir->input_dev);
-		} else if (ir->delay_timer_finished) {
+		} else if (ir->input_dev->timer_data) {
 			input_event(ir->input_dev, EV_KEY, keycode, 2);
 			input_sync(ir->input_dev);
 		}
 	} else {
-		ir->delay_timer_finished = 0;
+		ir->input_dev->timer_data = 0;
 		input_event(ir->input_dev, EV_KEY, keycode, 1);
 		input_sync(ir->input_dev);
 	}

@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
 
 
 /* called by the input driver after rep[REP_DELAY] ms */
-static void input_repeat_key(unsigned long parm)
+static void input_repeat_key(struct timer_list *t)
 {
-	struct infrared *ir = (struct infrared *) parm;
+	struct input_dev *dev = from_timer(dev, t, timer);
 
-	ir->delay_timer_finished = 1;
+	/* Key repeat started */
+	dev->timer_data = 1;
 }
 
 

@@ -365,8 +366,7 @@ int av7110_ir_init(struct av7110 *av7110)
 		input_free_device(input_dev);
 		return err;
 	}
-	input_dev->timer.function = input_repeat_key;
-	input_dev->timer.data = (unsigned long) &av7110->ir;
+	input_dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
 
 	if (av_cnt == 1) {
 		e = proc_create("av7110_ir", S_IWUSR, NULL, &av7110_ir_proc_fops);

include/linux/input.h +2 -0

index fb5e23c7ed98..dcd117bf1027 100644
@@ -70,6 +70,7 @@ struct input_value {
  * @repeat_key: stores key code of the last key pressed; used to implement
  *	software autorepeat
  * @timer: timer for software autorepeat
+ * @timer_data: timer data for software autorepeat overrides
  * @rep: current values for autorepeat parameters (delay, rate)
  * @mt: pointer to multitouch state
  * @absinfo: array of &struct input_absinfo elements holding information

@@ -152,6 +153,7 @@ struct input_dev {
 
 	unsigned int repeat_key;
 	struct timer_list timer;
+	unsigned long timer_data;
 
 	int rep[REP_CNT];
 

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Quoted 4 lines · Kees Cook

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

Quoted 4 lines

One input_dev user hijacks the input_dev software autorepeat timer to perform its own repeat management. However, there is no path back to the existing status variable, so add a generic one to the input structure and use that instead.

That is too bad and it should not be doing this. I'd rather av7110 used its own private timer for that.

Quoted 10 lines · Dmitry Torokhov

Quoted 8 lines · Kees Cook

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to perform its own repeat management. However, there is no path back to the existing status variable, so add a generic one to the input structure and use that instead.

Quoted 2 lines

That is too bad and it should not be doing this. I'd rather av7110 used its own private timer for that.

Yeah, that was a pretty weird case. I couldn't see how to avoid it, though. I didn't see a way to hook the autorepeat, but I'm not too familiar with the code here.

Quoted 15 lines · Kees Cook

Quoted 12 lines · Dmitry Torokhov

Quoted 8 lines · Kees Cook

In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly.

One input_dev user hijacks the input_dev software autorepeat timer to perform its own repeat management. However, there is no path back to the existing status variable, so add a generic one to the input structure and use that instead.

That is too bad and it should not be doing this. I'd rather av7110 used its own private timer for that.

Quoted 3 lines

Yeah, that was a pretty weird case. I couldn't see how to avoid it, though. I didn't see a way to hook the autorepeat, but I'm not too familiar with the code here.

You just need to manage the private timer in the driver and not mess up with the input core if input core's autorepeat does not provide the desired behavior...